fix: local overcommit file merges with existing .overcommit.yml file

sds / overcommit

A fully configurable and extendable Git hook manager

MIT License

3.92k stars 280 forks source link

fix: local overcommit file merges with existing .overcommit.yml file #815

Closed julesros closed 10 months ago

julesros commented 1 year ago

The .local-overcommit.yml did not work as intended. When a repo level .overcommit.yml and .local-overcommit.yml both existed, only the latter applied. Further, the tests only checked that each file was loaded, not that the configs included the expected overrides.

This PR refactors:

Specs:
- When .overcommit.yml exists, applies settings and has default settings still
- When both .local-overcommit.yml and .overcommit.yml exists has default settings and both files settings applied
Logic to merge the .overcommit.yml file to default settings and if it exists, .local-overcommit.yml file to both

julesros commented 1 year ago

@sds Greetings! I was using the feature I added earlier this year and realized it had a bug. Providing a fix here for your consideration.

simoleone commented 11 months ago

I just hit the same bug and found it rather confusing. Anything I can do to help get this merged?

julesros commented 11 months ago

I just hit the same bug and found it rather confusing. Anything I can do to help get this merged?

👋🏻 @simoleone , I need to take a look at where I left this. If it works as I had intended we can tag the maintainers for review I think. I'll try to get to it this weekend! If you'd like to test this branch locally as well and make sure it works as expected for the issue you found as well, that would probably be great.

simoleone commented 11 months ago

hey! Yup this works the way I think it should 😄

simoleone commented 10 months ago

Hey! Any way I can help out with this? 😄 . I know how free time goes with my own projects (or doesn't 🙃 ).

julesros commented 10 months ago

Hey! Any way I can help out with this? 😄 . I know how free time goes with my own projects (or doesn't 🙃 ).

Hi! Ah so sorry, yes, time is hard to find 🙃 If you've pulled down this branch and it looks like it works, then I think we can just go ahead and ask for a review from maintainers and request it to be merged in. I had wanted to clean it up more but if it works we can go with it.