fix(commit): resolve 'always_signoff' configuration and '-s' CLI issues

[AdrianDC]

Description

The always_signoff configuration or the CLI -s argument fail upon git commit call due to the passed syntax being git commit -- -s rather than git commit -s.

The -- is needed only on the commitizen CLI, and if no git argument is passed, the -- should not be forced.

signoff mechanic is deprecated, please use cz commit -- -s instead. fatal: /tmp/...: '/tmp/... is outside repository at '...'

Checklist

• [x] Add test cases to all the changes you introduce
• [x] Run ./scripts/format and ./scripts/test locally to ensure this change passes linter check and test
• [x] Test the changes on the local machine manually
• [ ] Update the documentation for the changes

Expected behavior

• Warning and successful if -s
• Warning and successful if -s --
• No warning and successful if -- -s
• No warning and successful if -- -s
• No warning and successful with always_signoff: true
• No warning and successful if -- with always_signoff: true
• No warning and successful if -s with always_signoff: true
• No warning and successful if -s -- with always_signoff: true
• No warning and successful if -- -s with always_signoff: true

Steps to Test This Pull Request

.cz.yaml:

---
commitizen:

  # commitizen configurations
  always_signoff: true

• cz c
• cz c --
• cz c -s
• cz c -s --
• cz c -- s

Additional context

Related to issue #1135 and #1146

Tested with the python:3.12 Docker image:

docker run -i --rm --entrypoint bash python:3.12 <<EOF
{
  git clone -b signoff https://github.com/AdrianDC/commitizen.git /tmp/commitizen
  cd /tmp/commitizen/
  pip install poetry
  poetry install
  ./scripts/format
  ./scripts/test
}
EOF

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 97.58%. Comparing base (120d514) to head (e305ef9). Report is 490 commits behind head on master.

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     

Flag	Coverage Δ
unittests	97.58% <100.00%> (+0.24%)	:arrow_up:

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

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[AdrianDC]

Just noticed we haven't marked -s as deprecated yet. We should probably do that. Would it be possible for you to include that in this PR? Thanks!

I'd actually be against deprecating -s, given it's the most used parameter after -m since years in our use cases, and I'd like to get a good adoption rate for either commitizen's cz c -s or git's git commit -s with my hooks.

Also, the always_signoff configuration I fixed in the first place through this MR requires this source code anyways.

---

If you're interested in my implementations of pre-commit + commitizen + prepare-commit-msg hooks, feel free to check : https://radiandevcore.gitlab.io/tools/pre-commit-crocodile/commitizen/#configurations-for-commitizen and the almost-auto-derived specifications : https://radiandevcore.gitlab.io/tools/pre-commit-crocodile/commits/

Initially meant to centralize the development features of my GitLab and Python related tools, it grew quickly into an automated adoption tool for pre-commit and my commitizen configurations.

I will then deploy the configurations progressively on our internal projects (simply pre-commit-crocodile --configure) and my developers will be able to use pre-commit-crocodile --enable easily on a day to day basis.

I'll probably use git commit -s on my side, especially with my automated type(scope): evaluators, and let developers prefer to use cz commit depending on their personal feelings, both giving a very similar result + same validation, and the CI/CD job component checks all commits either way.

[Lee-W]

Just noticed we haven't marked -s as deprecated yet. We should probably do that. Would it be possible for you to include that in this PR? Thanks!

I'd actually be against deprecating -s, given it's the most used parameter after -m since years in our use cases, and I'd like to get a good adoption rate for either commitizen's cz c -s or git's git commit -s with my hooks.

Also, the always_signoff configuration I fixed in the first place through this MR requires this source code anyways.

If you're interested in my implementations of pre-commit + commitizen + prepare-commit-msg hooks, feel free to check : https://radiandevcore.gitlab.io/tools/pre-commit-crocodile/commitizen/#configurations-for-commitizen and the almost-auto-derived specifications : https://radiandevcore.gitlab.io/tools/pre-commit-crocodile/commits/

Initially meant to centralize the development features of my GitLab and Python related tools, it grew quickly into an automated adoption tool for pre-commit and my commitizen configurations.

I will then deploy the configurations progressively on our internal projects (simply pre-commit-crocodile --configure) and my developers will be able to use pre-commit-crocodile --enable easily on a day to day basis.

I'll probably use git commit -s on my side, especially with my automated type(scope): evaluators, and let developers prefer to use cz commit depending on their personal feelings, both giving a very similar result + same validation, and the CI/CD job component checks all commits either way.

My thought on whether to keep these "commonly used" arguments is detailed https://github.com/commitizen-tools/commitizen/pull/1217#discussion_r1726179063. let's keep the discussion in one place 🙂

---

https://radiandevcore.gitlab.io/tools/pre-commit-crocodile/commitizen/ looks cool btw 🤩

[AdrianDC]

https://radiandevcore.gitlab.io/tools/pre-commit-crocodile/commitizen/ looks cool btw 🤩

Thanks !

Fine, reworking the commits series tomorrow.

[AdrianDC]

Reimplemented as discussed, documentations updated and tests too.

Deprecation commit on top of this MR pushed for v4 on MR #1221.

---

Understood the test_commit_command_shows_description_when_use_help_option failures I couldn't see locally, the test is restricted to Python 3.13, but the python:3.13.0rc1 image fails at poetry install for now.

[Lee-W]

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.

[AdrianDC]

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 pre-commit-crocodile users use the prerelease branch anyways,
until all commits are upstreamed, hence no rush : https://github.com/AdrianDC/commitizen/commits/prerelease/

(Tested the sync fork feature of GitHub, but it creates a merge commit rather than rebase, sad...)

[AdrianDC]

Hey ! Little up on this one :wink:

[AdrianDC]

Rebased, ready to roll :+1: