Skip to content
This repository was archived by the owner on Jun 23, 2023. It is now read-only.

Address various issues with the CLI script. #27

Merged
merged 1 commit into from
Mar 6, 2019
Merged

Conversation

radekk
Copy link
Contributor

@radekk radekk commented Mar 6, 2019

  • Do not throw an error when secrets were not found, refs Removed some code when no secrets are found #5
  • Return empty result set when no secrets were found
  • Throw an error only if error occurred:
    • Directory not found
    • CLI arguments not provided
  • Improve semantics on the JSON output:

OLD:

{
  "result": {
    "./src/filters/entropy.meter/pre.filters/css.selectors.js": [
      "(^([#.][a-z0-9_-]+){1,}$)|",
      "(^([#.]?[a-z0-9_-]+>){1,}([#.]?[a-z0-9_-]+)$)|"
    ]
  }
}

NEW:

{
  "result": [
    {
      "filepath": "./src/filters/entropy.meter/pre.filters/css.selectors.js",
      "secrets": [
        "(^([#.][a-z0-9_-]+){1,}$)|",
        "(^([#.]?[a-z0-9_-]+>){1,}([#.]?[a-z0-9_-]+)$)|"
      ]
    }
  ]
}

This PR breaks the compatibility since it changes the output format for CLI tool in JSON mode. It's going to be released as the minor version update.

Unverified

No user is associated with the committer email.
- Do not throw an error when secrets were not found, refs #5
- Return empty result set when no secrets were found
- Throw an error only if error occurred:
	- Directory not found
	- CLI arguments not provided
- Improve semantics on the JSON output:

OLD:
```
{
  "result": {
    "./src/filters/entropy.meter/pre.filters/css.selectors.js": [
      "(^([#.][a-z0-9_-]+){1,}$)|",
      "(^([#.]?[a-z0-9_-]+>){1,}([#.]?[a-z0-9_-]+)$)|"
    ]
  }
}
```

NEW:
```
{
  "result": [
    {
      "filepath": "./src/filters/entropy.meter/pre.filters/css.selectors.js",
      "secrets": [
        "(^([#.][a-z0-9_-]+){1,}$)|",
        "(^([#.]?[a-z0-9_-]+>){1,}([#.]?[a-z0-9_-]+)$)|"
      ]
    }
  ]
}
```
@radekk radekk self-assigned this Mar 6, 2019
@radekk radekk marked this pull request as ready for review March 6, 2019 14:42
@radekk radekk requested a review from a team March 6, 2019 14:42
Copy link

@esarafianou esarafianou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with only a comment: Shouldn't this be a major release since it has breaking changes?

@radekk
Copy link
Contributor Author

radekk commented Mar 6, 2019

LGTM with only a comment: Shouldn't this be a major release since it has breaking changes?

You're totally right, I'm not sure why I stuck to this minor release, obviously it needs to be a major one. Good catch 👍 Thank you for a review.

@radekk radekk merged commit 5817113 into master Mar 6, 2019
@radekk radekk deleted the cli-improvements branch March 6, 2019 22:00
radekk pushed a commit that referenced this pull request Mar 12, 2019
* Update to the latest LTS version of NodeJS

* A variety of small fixes to non-production files

- Don't flag as many ESLint errors in tests
- Don't commit `dist/`
- Make CLI tests set environment variables in a way that works on Windows
- Make fixture loading work on Windows
  - Path variables use \, but module resolution can use /

* Use newer string & array method in filters to make the code more readable

* Update README.MD file

Adding gitter button to allow people chat and discuss topics related to that tool.

* Switch to use lStat instead of Stat to cover symlinks as well. fixes #11 #8

* Set default node version to 8 instead of 4.

* Upgrade npm packages.

* Update Readme.

Adding link to Wiki and Gitter chat.

* Update README file

Add a hint for creating a webhook. It should always be the `application/json` content type.

* Add Auth0 Product Security team as codeowners

Auth0 Product Security team maintains repo-supervisor and should
be the final reviewer and approver of all pull requests.

* Upgrade packages and adjust compatibility (#26)

* Upgrade packages.
* Replace github library with octokit/rest.
* Fix linter errors.

* Address various issues with the CLI script. (#27)

- Do not throw an error when secrets were not found, refs #5
- Return empty result set when no secrets were found
- Throw an error only if error occurred:
	- Directory not found
	- CLI arguments not provided
- Improve semantics on the JSON output
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants