-
Notifications
You must be signed in to change notification settings - Fork 212
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
Bug: Global extend options will take precedence over local options #753
Comments
Hey! Thank you for the issue. I have prepared the fix: #754. Now |
Please, check out 1.6.17 for the fix |
@mrexox I am sorry to say but the recent fix may have exposed a different bug. The bug may just be our fault for how we implemented the For context, we currently have a The package's The issue is that the second level lefthook/internal/config/load.go Lines 116 to 118 in 8959135
Overall, was |
@ardelato , thank you for providing a great example of the configuration setup! I never thought of such cases with multiple If you used to share the configuration I can suggest you the lefthook's I will try to implement recursive merging of extends but I think it may take some time. I'll let you know if I find a blocker. |
Hey @ardelato! It was easier than I thought. I will prepare a new version with this fix soon |
Hot damn! I would have been just fine if this just went on your radar but to address it so quickly makes me really appreciate this repo and the work you guys are doing 🙇. That said, the new version does seem to have fixed both issues now 🙌. Thank you @mrexox. |
🔧 Summary
We recently came across some unexpected behavior when trying to make use of the
exclude_tags
option in alefthook-local.yml
.We had already declared a
exclude_tags
option in one of theextends
files included through the global config, but we wanted to overwrite it through alefthook-local.yml
config.We discovered that the
extends
files will take precedence over thelefthook-local.yml
config because it gets re-applied at this line:lefthook/internal/config/load.go
Lines 116 to 117 in 8959135
Steps to reproduce
I added a test to
load_test.go
to reproduce the issue: master...ardelato:bug-repro--extends-takes-precendence-over-localExpected results
I expected all options from the
lefthook-local.yml
config to take precedence over everything else. Even more so because of this comment:lefthook/internal/config/load.go
Lines 97 to 102 in 8959135
Actual results
Options from the
extends
files will take precedence.Possible Solution
A potential fix could involve modifying the merge process to ensure that
extends
files do not get reapplied. This could be achieved by filtering out previously appliedextends
files during the merging process to prevent them from being in the list whenextends
is called after merging each config source:The text was updated successfully, but these errors were encountered: