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(commit): add --write-message-to-file option #731

Merged
merged 11 commits into from
May 1, 2023
Merged

feat(commit): add --write-message-to-file option #731

merged 11 commits into from
May 1, 2023

Conversation

crai0
Copy link
Contributor

@crai0 crai0 commented Apr 28, 2023

Description

This pull request resolves #689 by implementing my proposal for it.

Checklist

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

Expected behavior

See #689 (comment).

Steps to Test This Pull Request

New option:

  1. Run cz commit --write-message-to-file message.txt in a Git repository with staged changes and complete interactive dialog
  2. The content of message.txt should be the generated commit message
  3. The other behavior of cz commit is unchanged

Git hook:

  1. Follow the tutorial in docs/tutorials/auto_prepare_commit_message.md in a Git repository with staged changes
  2. When running git commit or git commit -m "...", the usual cz commit interactive dialog runs
  3. After completing the dialog (and exiting the GIT_EDITOR if it opened), a new commit with the generate commit message is created

Additional context

These changes supersede #249 and #250.

@crai0 crai0 changed the title Write message to file feat(commit): add --write-message-to-file option Apr 28, 2023
@codecov
Copy link

codecov bot commented Apr 28, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.04 ⚠️

Comparison is base (6096b68) 97.38% compared to head (f04a719) 97.35%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #731      +/-   ##
==========================================
- Coverage   97.38%   97.35%   -0.04%     
==========================================
  Files          42       42              
  Lines        2029     2041      +12     
==========================================
+ Hits         1976     1987      +11     
- Misses         53       54       +1     
Flag Coverage Δ
unittests 97.35% <100.00%> (-0.04%) ⬇️

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

Impacted Files Coverage Δ
commitizen/__version__.py 100.00% <100.00%> (ø)
commitizen/changelog.py 99.50% <100.00%> (-0.50%) ⬇️
commitizen/cli.py 94.20% <100.00%> (+0.08%) ⬆️
commitizen/commands/changelog.py 98.94% <100.00%> (+0.03%) ⬆️
commitizen/commands/commit.py 98.57% <100.00%> (+0.13%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

docs/commit.md Show resolved Hide resolved
@woile
Copy link
Member

woile commented Apr 29, 2023

Great work! 💪🏻 thanks!

@crai0
Copy link
Contributor Author

crai0 commented Apr 29, 2023

i noticed that it's not possible to use the --retry option with this workflow. if another hook aborts the commit after running git commit for example, the message generated with commitizen is lost. i would like to propose a follow-up issue in which the retry behavior can be configured as a default and a new --no-retry flag is added to override the configuration.

commitizen/commands/commit.py Outdated Show resolved Hide resolved
commitizen/cli.py Show resolved Hide resolved
@woile
Copy link
Member

woile commented Apr 29, 2023

The --no-retry would be a breaking change, I'd rather keep the API. The --retry has been requested a lot, so I imagine a lot of people are already using it, and I'd rather avoid changing it.

Is it not possible to run other checks and then the commit?

Is there a way to refactor the code in a non-breaking change?

But let's open another issue and discuss it there.

@crai0
Copy link
Contributor Author

crai0 commented Apr 29, 2023

this would not be a breaking change if it can be configured and isn't be enabled by default. i'll open an issue for it and describe the feature in a backward compatible manner.

@crai0
Copy link
Contributor Author

crai0 commented Apr 30, 2023

i had to remove the hook i was using because i added it on accident but otherwise i think this can be merged, right?

@crai0
Copy link
Contributor Author

crai0 commented Apr 30, 2023

never mind, i couldn't stand the way the hook worked before so i made the following changes:

  • have two separate hooks (prepare-commit-msg and post-commit) that can be downloaded from this repo
  • the hooks are written in python (which is actually more cross-compatible than bash)
  • the hooks check if commitizen is even installed
  • it's possible to make commits using commitizen using both git commit and cz commit
  • the hooks have a similar retry feature to the one that commitizen has and they're interoperable so that cz commit --retry can be used to retry the last commit made via git commit and vice versa
  • the tutorial now contains an installation guide and a feature overview

@woile
Copy link
Member

woile commented Apr 30, 2023

That's nice! Thanks for taking the time to build this. I'll take a look at it tomorrow

@woile woile merged commit 0354a9d into commitizen-tools:master May 1, 2023
@crai0 crai0 deleted the write-message-to-file branch May 1, 2023 14:36
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.

Is there any way to use this in a prepare-commit-msg hook?
2 participants