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

Improve docs for plugins based on RegexBasedDetector #170

Closed
dgzlopes opened this issue May 10, 2019 · 4 comments
Closed

Improve docs for plugins based on RegexBasedDetector #170

dgzlopes opened this issue May 10, 2019 · 4 comments

Comments

@dgzlopes
Copy link
Contributor

dgzlopes commented May 10, 2019

Hello! I thought (after reading contributing.md [0]) that maybe we should apply another format when adding a new plugin based on RegexBasedDetector [1]. Adding one point for each one looks like too much as they are all (Slack and AWS as an example) the same but with a different regex.

Maybe something like this?

  • RegexDetector: checks for all keys matching a regex (Artifactory, AWS, Slack...)

[0] https://github.com/Yelp/detect-secrets/blob/master/CONTRIBUTING.md
[1]

class RegexBasedDetector(BasePlugin):

@dgzlopes dgzlopes changed the title [Idea] Improve docs for plugins based on RegexBasedDetector Improve docs for plugins based on RegexBasedDetector May 10, 2019
@domanchi
Copy link
Contributor

Hey @dgzlopes,

Could you clarify what you mean? In my understanding, Slack and AWS regex tokens are quite distinct, and use different regexes.

@dgzlopes
Copy link
Contributor Author

Yeah sure! I mean that as they are all based on the same detector and as the only variation is the regex, maybe spending one line per detector for saying the 'same' is too much! As an example:

  • ArtifactoryDetector: checks to see if Artifactory credentials are present. (This actually exists on the docs)

  • SlackDetector: checks to see if Slack credentials are present.

  • AWSDetector: checks to see if AWS credentials are present.

  • StripeDetector: checks to see if Stripe credentials are present.

That's why I thought that maybe grouping them under the same 'RegexBasedDetectors' dot could be useful. And if some detector "implements" something different or needs some special clarification, It can be moved to its own bullet point.

@domanchi
Copy link
Contributor

Oh, gotcha! You're talking about DRYer documentation.

Sure, feel free to cut out a PR if you think this could be clearer.

@dgzlopes
Copy link
Contributor Author

#177 merged. Closing this one.

killuazhu pushed a commit to IBM/detect-secrets that referenced this issue May 28, 2020
Yelp#170)

This reverts commit 8aa90ee.

Temporarily revert the offending commit to prevent scan missing on old git version. The upstream issue is Yelp#220

After this quick revert, we lose the feature of scanning multiple repos at once. But it should block some of our user from using detect-secrets with old version git.

We should investigate a better fix later.
killuazhu pushed a commit to IBM/detect-secrets that referenced this issue Jul 9, 2020
Yelp#170)

This reverts commit 8aa90ee.

Temporarily revert the offending commit to prevent scan missing on old git version. The upstream issue is Yelp#220

After this quick revert, we lose the feature of scanning multiple repos at once. But it should block some of our user from using detect-secrets with old version git.

We should investigate a better fix later.
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

No branches or pull requests

3 participants