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

feat(config): allow overriding config from environment #10

Merged
merged 1 commit into from
Dec 22, 2023

Conversation

noirbizarre
Copy link
Contributor

Hello 👋🏼

First I apologize because I read the CONTRIBUTING.md after doing my PR so there is no open issue and no discussion for this. But any critism or refusal is welcome here 🙏🏼 (better late then never).

This PR allows to override some settings (all but dependency_mapping) using SYNC_PRE_COMMIT_LOCK_* environment variables.

There are multiple use case for this, but in my case I currently have 2:

  • Explicitly disabling on project I contribute on but without being maintainer (meaning that I don't want to touch anything except my contribution, even if the pre-commit setting is not in sync)
  • testing pre-commit configs (in my case, snapshot testing some copier templates, I want to be able to disable it during the test)

Note: I plan on doing another contribution real soon, but I promise I'll open an issue before to discuss this

Copy link

codecov bot commented Dec 19, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (45a5eb7) 94.23% compared to head (af01f65) 94.49%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #10      +/-   ##
==========================================
+ Coverage   94.23%   94.49%   +0.25%     
==========================================
  Files           9        9              
  Lines         434      454      +20     
  Branches       74       79       +5     
==========================================
+ Hits          409      429      +20     
  Misses         21       21              
  Partials        4        4              
Flag Coverage Δ
unittests 94.49% <100.00%> (+0.25%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Owner

@GabDug GabDug left a comment

Choose a reason for hiding this comment

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

Quick review, with some typos and comment on the env var cast.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
src/sync_pre_commit_lock/config.py Outdated Show resolved Hide resolved
@GabDug
Copy link
Owner

GabDug commented Dec 19, 2023

Hey!
Don't worry, it's not an issue to not have discussed it before -- the goal is more to avoid working on out-of-scope MRs or duplicated effort: the contributing guide may be clearer.

This is a very welcomed improvement, thanks!

@noirbizarre
Copy link
Contributor Author

There it is, PR updated.

On the contributing guide, this is always hard to find the right balance between being open enough for people to easily contribute and strict enough for this to be sustainable. I contribute a lot, try to follow them but I still encounter some issues sometimes so I am careful because I can understand the pressure of receiving a not welcome PR 😅

Copy link
Owner

@GabDug GabDug left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, sorry for the delay :)

@GabDug GabDug merged commit d72b7ca into GabDug:main Dec 22, 2023
25 checks passed
@noirbizarre noirbizarre deleted the feature/disable-from-env branch December 22, 2023 14:09
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

2 participants