-
-
Notifications
You must be signed in to change notification settings - Fork 186
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
docs: improve updating pre-commit documentation #1361
Conversation
12rambau
commented
Oct 6, 2023
- there was a inconsistence between the inline and rej default. as it's inline I changed the value in the corrected section
- as users won't need the 2 options at once, I split the 2 hooks.
- there was a inconsistence between the inline and rej default. as it's inline I changed the value in the corrected section - as users won't need the 2 options at once, I split the 2 hooks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution and also for catching the outdated default value for the --conflict
flag, @12rambau! 🙏
I have only a few minor remarks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's good to keep both examples together. In a shared project, one user might use rej mode, while others use inline mode. In any case, you don't want those files committed.
If you want to keep them split because of clarity, at least please explain this point too.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1361 +/- ##
==========================================
+ Coverage 97.19% 97.46% +0.26%
==========================================
Files 48 48
Lines 4280 4537 +257
==========================================
+ Hits 4160 4422 +262
+ Misses 120 115 -5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
The current documentation is confusing. At first it is stated that |
In that case, the default is inline. |
Co-authored-by: Sigurd Spieckermann <[email protected]>
Co-authored-by: Sigurd Spieckermann <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some final polishing. 😉
Oh, and CI is failing because Prettier wants a line wrap in the description: >
found Copier ...
... |
Co-authored-by: Sigurd Spieckermann <[email protected]>
Fixed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect, thanks! 👌