Skip to content

Conversation

bufferoverflow
Copy link
Contributor

Closes #1384

Description:

Document actual behavior

Checklist:

  • Does your PR pass tests?
  • Have you written new tests for your changes?
  • Have you lint your code locally prior to submission?

README.md Outdated
# the base rules take precedence over the extended rules. I.e., if there are
# duplicate rules in both the base configuration and the extended configuration
# the base rules will override the extended rules.
# the extended rules take precedence over the base rules. I.e., if there are
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't strictly true, as #1556 explicitly causes our config to overwrite values in the extended config.

I think we need better language to describe the relationship. "Base" is used ambiguously in a few places, e.g., in the code "base" refers to the extended rule here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, you are right. I somehow struggle a bit to suggest a proper text here. @rgmz Do you have a good suggestion?

Copy link

@amithm7 amithm7 Nov 28, 2024

Choose a reason for hiding this comment

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

@bufferoverflow @rgmz

  • "custom" would be a better word for the user-defined config. "base" can cause confusion, as it maybe misunderstood to refer the default config (gitleaks provided).
  • Users would generally want "custom" user-defined config rules to take precedence (override) over the "inherited" rules from the extended / parent / default config.
  • If there are exceptions on this overriding, it maybe noted.

I hope that these word suggestions help to form a better text here. Let me know if I can help.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@zricethezav @rgmz @amithm7 I rewrote the README.md regarding extending rules, please take a look.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ping

@bufferoverflow
Copy link
Contributor Author

@rgmz @zricethezav please close this PR if you see no value.

@rgmz
Copy link
Contributor

rgmz commented Feb 10, 2025

That's up to @zricethezav: I have no permissions. Personally, I think it's a good change.

@zricethezav zricethezav merged commit 398d0c4 into gitleaks:master Feb 11, 2025
1 check passed
@zricethezav
Copy link
Collaborator

fell off the radar, sorry about that. Thanks @bufferoverflow and @rgmz

alayne222 pushed a commit to alayne222/gitleaks that referenced this pull request May 28, 2025
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.

Documentation for the [extend] file is wrong
4 participants