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.

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

Adding feature to ignore patterns/keywords #213

Merged
merged 3 commits into from
Jul 24, 2020

Conversation

steeve85
Copy link
Contributor

Based on user feedback where Talisman was detecting some keyword frequently used like key, I decided to look at the source code to see if we can add an allowed list of patterns/keywords to avoid having users ignoring file content detector for example.

This appears to be a feature that other Talisman users are also interested to have if I refer to issue #39.

Based on this comment, I added the feature to allow keyword at the repository level and also at the file level.

.talismanrc file can now look like this:

fileignoreconfig:
- filename: test
  checksum: 340a057ace3d476d4a4feb8f6e3f84d6b9ca5bf8963a20d2ebe29be3f51297d5
  ignore_detectors: []
  allowed_patterns: [key, pass]
scopeconfig: []
allowed_patterns:
- key
- pass

Please let me know what you think of this MVP version of the feature. I can improve it based on your feedback and add unit tests too.

@svishwanath-tw
Copy link
Collaborator

Great PR.
I want to see 2 more things in this PR:

  1. Tests for both global and file level allowed patterns
  2. TALISMAN_ALLOWED_PATTERN could be just '' (as in replace with blank string instead of TALISMAN_ALLOWED_PATTERN (this could lead to a mysterious bug if someone set TALISMAN_ALLOWED_PATTERN as a custom search pattern for the repo)

@svishwanath-tw svishwanath-tw merged commit 5eb039a into thoughtworks:master Jul 24, 2020
@harinee
Copy link
Collaborator

harinee commented Jul 24, 2020

Could I please request you to update README with this new feature as well? @steeve85

@steeve85
Copy link
Contributor Author

As this PR has already been merged, I will create a new PR today for the 2 changes that @svishwanath-tw requested and the README update. Thanks

steeve85 added a commit to steeve85/talisman that referenced this pull request Jul 24, 2020
svishwanath-tw pushed a commit that referenced this pull request Jul 25, 2020
I have updated the README
I added a Test for allowed pattern at the file and repository level
I have updated the string used to replace allowed keywords
I added omitempty option on FileIgnoreConfig.Checksum as I believe this is not a required parameter (This might need additional testing 👀 cc @svishwanath-tw @harinee )
harinee pushed a commit to harinee/talisman that referenced this pull request Aug 9, 2020
harinee pushed a commit to harinee/talisman that referenced this pull request Aug 9, 2020
I have updated the README
I added a Test for allowed pattern at the file and repository level
I have updated the string used to replace allowed keywords
I added omitempty option on FileIgnoreConfig.Checksum as I believe this is not a required parameter (This might need additional testing 👀 cc @svishwanath-tw @harinee )
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants