Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove old-style config for exclude-entropy-patterns #282

Merged
merged 9 commits into from
Nov 19, 2021
Merged

Remove old-style config for exclude-entropy-patterns #282

merged 9 commits into from
Nov 19, 2021

Conversation

sushantmimani
Copy link
Contributor

@sushantmimani sushantmimani commented Nov 11, 2021 •
edited
Loading

Fix tests

To help us get this pull request reviewed and merged quickly, please be sure to include the following items:

  • Tests (if applicable)
  • Documentation (if applicable)
  • Changelog entry
  • A full explanation here in the PR description of the work done

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Tests
  • Other

Backward Compatibility

Is this change backward compatible with the most recently released version? Does it introduce changes which might change the user experience in any way? Does it alter the API in any way?

  • Yes (backward compatible)
  • No (breaking changes)
    Old style exluded-entropy-patterns will no longer be supported and will raise an error

Issue Linking

Closes #264

What's new?

Old style exluded-entropy-patterns will no longer be supported and will raise an error. These must now follow the TOML array of tables format and can only be specified in the config file

@sushantmimani sushantmimani marked this pull request as ready for review November 11, 2021 20:10
@sushantmimani sushantmimani requested a review from a team as a code owner November 11, 2021 20:10
@sushantmimani sushantmimani force-pushed the update-exclude-entropy-path branch from ad37372 to db00e32 Compare November 11, 2021 20:14
rbailey-godaddy
rbailey-godaddy approved these changes Nov 11, 2021
View reviewed changes
Copy link
Contributor

@rbailey-godaddy rbailey-godaddy left a comment

Choose a reason for hiding this comment

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

LGTM

rbailey-godaddy
rbailey-godaddy approved these changes Nov 12, 2021
View reviewed changes
Copy link
Contributor

@rbailey-godaddy rbailey-godaddy left a comment

Choose a reason for hiding this comment

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

It still looks good. ;-) I made some nit-picking editorial comments on the documentation, which you can take or leave.

docs/source/features.rst Outdated Show resolved Hide resolved
docs/source/features.rst Outdated Show resolved Hide resolved
tartufo/cli.py Show resolved Hide resolved
@sushantmimani sushantmimani force-pushed the update-exclude-entropy-path branch 2 times, most recently from 96882f4 to d0fbbaa Compare November 15, 2021 18:23
tarkatronic
tarkatronic suggested changes Nov 16, 2021
View reviewed changes
Copy link
Contributor

@tarkatronic tarkatronic left a comment

Choose a reason for hiding this comment

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

Nothing too serious. But I want to be certain we retain the old regex loading functionality for the default_regexts.json loading. And either utilize the new MatchType and Scope enums or remove them since they're not currently used at all. 😄

docs/source/features.rst Show resolved Hide resolved
tartufo/config.py Outdated Show resolved Hide resolved
tartufo/types.py Outdated Show resolved Hide resolved
tartufo/util.py Show resolved Hide resolved
@sushantmimani sushantmimani force-pushed the update-exclude-entropy-path branch from d0fbbaa to fd48661 Compare November 16, 2021 21:34
tarkatronic
tarkatronic suggested changes Nov 16, 2021
View reviewed changes
docs/source/features.rst Outdated Show resolved Hide resolved
docs/source/features.rst Outdated Show resolved Hide resolved
tartufo/config.py Outdated Show resolved Hide resolved
tartufo/scanner.py Outdated Show resolved Hide resolved
tartufo/scanner.py Show resolved Hide resolved
docs/source/features.rst Show resolved Hide resolved
@tarkatronic
Copy link
Contributor

Another thought here: We've now documented that this option is only available in the config file, and not from the CLI. But it's still being added as a @cli.option. So when the user runs tartufo --help they will see it as a potential option on the command line. We should see if we can remove it from there, and still use it as a config-file option.

tarkatronic
tarkatronic suggested changes Nov 18, 2021
View reviewed changes
Copy link
Contributor

@tarkatronic tarkatronic left a comment

Choose a reason for hiding this comment

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

Just a couple of small updates left, viewing this from the perspective of being a library/API first, with a CLI exposing it second. Great work @sushantmimani!

docs/source/features.rst Outdated Show resolved Hide resolved
tartufo/config.py Outdated Show resolved Hide resolved
tartufo/scanner.py Show resolved Hide resolved
@tarkatronic tarkatronic linked an issue Nov 18, 2021 that may be closed by this pull request
Remove old-style config for exclude-entropy-patterns #264
Closed
sushantmimani and others added 2 commits November 18, 2021 14:30
@sushantmimani @tarkatronic
Update docs/source/features.rst
60ac381 
Co-authored-by: Joey Wilhelm <tarkatronic@gmail.com>
@sushantmimani
Address review comments
998325c
tarkatronic
tarkatronic approved these changes Nov 19, 2021
View reviewed changes
Copy link
Contributor

@tarkatronic tarkatronic left a comment

Choose a reason for hiding this comment

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

🎉 🚢 :shipit:

@tarkatronic tarkatronic merged commit 8348951 into godaddy:v3.x Nov 19, 2021
@tarkatronic tarkatronic mentioned this pull request Nov 19, 2021
Closed
@sushantmimani sushantmimani deleted the update-exclude-entropy-path branch November 19, 2021 17:49
@tarkatronic tarkatronic added this to the Version 3.0 milestone Nov 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rbailey-godaddy rbailey-godaddy rbailey-godaddy approved these changes

@tarkatronic tarkatronic tarkatronic approved these changes

@mayuriesha mayuriesha Awaiting requested review from mayuriesha

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Version 3.0
Development

Successfully merging this pull request may close these issues.

Remove old-style config for exclude-entropy-patterns
3 participants
@sushantmimani @tarkatronic @rbailey-godaddy