fix(commit): resolve 'always_signoff' configuration and '-s' CLI issues#1206
fix(commit): resolve 'always_signoff' configuration and '-s' CLI issues#1206Lee-W merged 5 commits intocommitizen-tools:masterfrom
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1206 +/- ##
==========================================
+ Coverage 97.33% 97.58% +0.24%
==========================================
Files 42 55 +13
Lines 2104 2607 +503
==========================================
+ Hits 2048 2544 +496
- Misses 56 63 +7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
db79133 to
f903f01
Compare
I'd actually be against deprecating Also, the If you're interested in my implementations of pre-commit + commitizen + prepare-commit-msg hooks, Initially meant to centralize the development features of my GitLab and Python related tools, I will then deploy the configurations progressively on our internal projects (simply I'll probably use |
My thought on whether to keep these "commonly used" arguments is detailed #1217 (comment). let's keep the discussion in one place 🙂 https://radiandevcore.gitlab.io/tools/pre-commit-crocodile/commitizen/ looks cool btw 🤩 |
Thanks ! Fine, reworking the commits series tomorrow. |
2742424 to
d211348
Compare
|
Reimplemented as discussed, documentations updated and tests too. Deprecation commit on top of this MR pushed for v4 on MR #1221. Understood the |
|
Hi @AdrianDC , just a head up. I'm aware of these changes, but I am overwhelmed these days. I'll try my best to take a look ASAP. |
|
No worries @Lee-W , same here since 1+ year, only had time to work on my GitLab related tools the last weeks. For the time being, my developers and (Tested the sync fork feature of GitHub, but it creates a merge commit rather than rebase, sad...) |
|
Hey ! Little up on this one 😉 |
Lee-W
left a comment
There was a problem hiding this comment.
looks good to me! Once the conflict is resolved, I think we're close to merge
If 'always_signoff' is enabled in configurations, or '-s' is used alone on the CLI, the following errors arise due to 'git commit' argument failures : > signoff mechanic is deprecated, please use `cz commit -- -s` instead. > fatal: /tmp/...: '/tmp/... is outside repository at '...' Signed-off-by: Adrian DC <radian.dc@gmail.com>
Signed-off-by: Adrian DC <radian.dc@gmail.com>
Signed-off-by: Adrian DC <radian.dc@gmail.com>
Signed-off-by: Adrian DC <radian.dc@gmail.com>
…nd sources
Details: The git sources folder ownership may be detected as dubious if running
in a container with sources mounted to work on fixes and tests,
breaking 'test_find_git_project_root' and 'test_get_commits_with_signature'
> commitizen.exceptions.GitCommandError: fatal: detected dubious ownership in repository at '...'
---
Signed-off-by: Adrian DC <radian.dc@gmail.com>
|
Rebased, ready to roll 👍 |
Lee-W
left a comment
There was a problem hiding this comment.
LGTM. I'll keep it for a few days so that others can take a look. Thanks for your help!
Description
The
always_signoffconfiguration or the CLI-sargument fail upongit commitcalldue to the passed syntax being
git commit -- -srather thangit commit -s.The
--is needed only on the commitizen CLI, and if no git argument is passed, the--should not be forced.Checklist
./scripts/formatand./scripts/testlocally to ensure this change passes linter check and testExpected behavior
-s-s ---- -s-- -salways_signoff: true--withalways_signoff: true-swithalways_signoff: true-s --withalways_signoff: true-- -swithalways_signoff: trueSteps to Test This Pull Request
.cz.yaml:
cz ccz c --cz c -scz c -s --cz c -- sAdditional context
Related to issue #1135 and #1146
Tested with the
python:3.12Docker image: