-
-
Notifications
You must be signed in to change notification settings - Fork 301
feat(commands/commit): apply prepare-commit-msg hook #250
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(commands/commit): apply prepare-commit-msg hook #250
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@ ## master #250 +/- ## ========================================== + Coverage 97.92% 97.97% +0.05% ========================================== Files 39 43 +4 Lines 1395 1432 +37 ========================================== + Hits 1366 1403 +37 Misses 29 29
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Hi, although I've not yet had the time to fully review the code, I just test it on my local machine and failed.
This is the .pre-commit-config.yaml I used.
default_stages: [push] repos: - repo: https://github.com/pre-commit/pre-commit-hooks rev: v3.1.0 hooks: - id: check-vcs-permalinks - id: end-of-file-fixer - id: trailing-whitespace args: [--markdown-linebreak-ext=md] - id: debug-statements - id: no-commit-to-branch - repo: https://github.com/saygox/commitizen rev: 09fa312b7846850f44f7860632685189e37c1bec hooks: - id: commitizen-prepare-commit-msg stages: [prepare-commit-msg
The command I used.
mkdir test cd test git init # add .pre-commit-config.yaml pre-commit install -t prepare-commit-msg git commt
The error message
File "/.../.cache/pre-commit/repof8ns4eua/py_env-python3/bin/cz", line 8, in <module> sys.exit(main()) File "/.../.cache/pre-commit/repof8ns4eua/py_env-python3/lib/python3.7/site-packages/commitizen/cli.py", line 286, in main args.func(conf, vars(args))() File "/.../.cache/pre-commit/repof8ns4eua/py_env-python3/lib/python3.7/site-packages/commitizen/commands/commit.py", line 94, in __call__ m = self.prompt_commit_questions() File "/.../.cache/pre-commit/repof8ns4eua/py_env-python3/lib/python3.7/site-packages/commitizen/commands/commit.py", line 63, in prompt_commit_questions answers = questionary.prompt(questions, style=cz.style) File "/.../.cache/pre-commit/repof8ns4eua/py_env-python3/lib/python3.7/site-packages/questionary/prompt.py", line 97, in prompt answer = question.unsafe_ask(patch_stdout) File "/.../.cache/pre-commit/repof8ns4eua/py_env-python3/lib/python3.7/site-packages/questionary/question.py", line 59, in unsafe_ask return self.application.run() File "/.../.cache/pre-commit/repof8ns4eua/py_env-python3/lib/python3.7/site-packages/prompt_toolkit/application/application.py", line 816, in run self.run_async(pre_run=pre_run, set_exception_handler=set_exception_handler)
- Python version: 3.7
- OS: macOS 10.15
ShaneKosieradzki
commented
Jan 5, 2021
Glad to see this is being implemented as it is the biggest thing preventing us from adopting this technology. Is there a timeline for this PR?
Also @Lee-W not sure if this error occurred when you were creating the comment on this PR but you have a typo in the stages section of your .pre-commit-config.yaml for the commitizen-prepare-commit-msg hook. You are missing a closing ]. Perhaps that is what caused your error when running on your local machine.
@ShaneKosieradzki Thanks for reminding me! But I still encounter an error on my local machine. Did you succeed on your local machine?
Traceback (most recent call last):
File "/Users/weilee/.cache/pre-commit/repof8ns4eua/py_env-python3/bin/cz", line 8, in <module>
sys.exit(main())
File "/Users/weilee/.cache/pre-commit/repof8ns4eua/py_env-python3/lib/python3.8/site-packages/commitizen/cli.py", line 286, in main
args.func(conf, vars(args))()
File "/Users/weilee/.cache/pre-commit/repof8ns4eua/py_env-python3/lib/python3.8/site-packages/commitizen/commands/commit.py", line 95, in __call__
m = self.prompt_commit_questions()
File "/Users/weilee/.cache/pre-commit/repof8ns4eua/py_env-python3/lib/python3.8/site-packages/commitizen/commands/commit.py", line 63, in prompt_commit_questions
answers = questionary.prompt(questions, style=cz.style)
File "/Users/weilee/.cache/pre-commit/repof8ns4eua/py_env-python3/lib/python3.8/site-packages/questionary/prompt.py", line 97, in prompt
answer = question.unsafe_ask(patch_stdout)
File "/Users/weilee/.cache/pre-commit/repof8ns4eua/py_env-python3/lib/python3.8/site-packages/questionary/question.py", line 59, in unsafe_ask
return self.application.run()
File "/Users/weilee/.cache/pre-commit/repof8ns4eua/py_env-python3/lib/python3.8/site-packages/prompt_toolkit/application/application.py", line 814, in run
return loop.run_until_complete(
File "/Users/weilee/.pyenv/versions/3.8.5/lib/python3.8/asyncio/base_events.py", line 616, in run_until_complete
return future.result()
File "/Users/weilee/.cache/pre-commit/repof8ns4eua/py_env-python3/lib/python3.8/site-packages/prompt_toolkit/application/application.py", line 781, in run_async
return await _run_async2()
File "/Users/weilee/.cache/pre-commit/repof8ns4eua/py_env-python3/lib/python3.8/site-packages/prompt_toolkit/application/application.py", line 763, in _run_async2
result = await _run_async()
File "/Users/weilee/.cache/pre-commit/repof8ns4eua/py_env-python3/lib/python3.8/site-packages/prompt_toolkit/application/application.py", line 694, in _run_async
with self.input.raw_mode(), self.input.attach(
File "/Users/weilee/.pyenv/versions/3.8.5/lib/python3.8/contextlib.py", line 113, in __enter__
return next(self.gen)
File "/Users/weilee/.cache/pre-commit/repof8ns4eua/py_env-python3/lib/python3.8/site-packages/prompt_toolkit/input/vt100.py", line 161, in _attached_input
loop.add_reader(fd, callback)
File "/Users/weilee/.pyenv/versions/3.8.5/lib/python3.8/asyncio/selector_events.py", line 332, in add_reader
return self._add_reader(fd, callback, *args)
File "/Users/weilee/.pyenv/versions/3.8.5/lib/python3.8/asyncio/selector_events.py", line 261, in _add_reader
self._selector.register(fd, selectors.EVENT_READ,
File "/Users/weilee/.pyenv/versions/3.8.5/lib/python3.8/selectors.py", line 522, in register
self._selector.control([kev], 0, 0)
OSError: [Errno 22] Invalid argument@Lee-W it worked for me (Ubuntu 18.04.5; python 3.6.9; pre-commit 2.13.0) using pretty much your steps (with the closing ']' fix).
Exact steps:
- make test
- cd test
- python3 -m venv venv
- source venv/bin/activate
- pip install pre-commit
- vim .pre-commit-config.yaml
repos: - repo: https://github.com/saygox/commitizen rev: 09fa312b7846850f44f7860632685189e37c1bec hooks: - id: commitizen-prepare-commit-msg stages: [prepare-commit-msg]
- git init
- git add .pre-commit-config.yaml
- pre-commit install --install-hooks -t prepare-commit-msg
- git commit
I did encounter one potential issue in playing around a bit, however, after successfully commiting.
- git commit --amend
commitizen prepare commit msg............................................No files added to staging!
Failed
- hook id: commitizen-prepare-commit-msg
- exit code: 11
I'd like to see this issue/PR continued -- note that it would also "solve", or at least provide a viable alternative to, #248 since you are actually using git commit and therefore any flags or configs that you want for that, such as -S or --signoff.
Hi @lobotmcj , I still encounter the same error on my mac. Still have no idea on how this could be fixed. In addition, we might need to take care of windows cases as well
@woile Do you have any though on this one?
Hi @saygox thanks for your update. Could you please help us rebase the latest change from the master branch? I've tried to do so but fail to pass the test. Also, I'm wondering is it excepted to call git.commit twice on line 143 in commitizen/commands/commit.py
Hi @Lee-W san.
I tried to rebase this branch from HEAD. but my operation is not good so I can't push it.
So I push the rebased branch to https://github.com/saygox/commitizen/tree/feature/prepare-commit-msg2 .
sorry to the line 143's git.commit is miss. prepare-commit-msg2 branch is fixed it.
Would you mind to tell me how to merge a new rebased one to this request ? (or need to create a new push request?)
Hi @saygox , have you try git push origin feature/prepare-commit-msg -f?
1a3ffb6 to
cc7066c
Compare
commitizen/commands/commit.py
Outdated
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.
nitpick: I'd suggest using default_message
1a947d1 to
04b860d
Compare
f4ac267 to
3a15639
Compare
@saygox Thanks for your hard work. I'm a bit busy lately, but I'll start digging into it these days.
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 hard work! I just tried it on my local machine and git commit with the pre-commit hook works well 🎉 but I just found out the if you run cz commit it will trigger the questionary twice as under the hook cz commit calls git commit as well. I think we might need to address this issue.
Also, I'm thinking of making wrap_studio a package.
wrap_studio/
__init__.py
unix.py
windows.py
linux.py
What do you think?
commitizen/wrap_stdio_linux.py
Outdated
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.
As this is not used, please remove this line
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.
line 6 is not need. I remove it.
line 3, 4 are needs for git hub release test. this details will be explained in the next item.
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.
Why do we need to check this in both commitizen/wrap_stdio.py and here?
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.
The coverage of pytest is check all files.
if it is removed from wrap_stdio_linux.py, the git hub release test for mac os is not passed.
---------- coverage: platform darwin, python 3.7.3-final-0 -----------
Name Stmts Miss Cover Missing
------------------------------------------------------------------------------------------
:
commitizen/wrap_stdio_linux.py 6 2 67% 12, 16
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.
and same reason.
wrap_stdio_unix.py is checked on the linux environment.
---------- coverage: platform linux, python 3.8.10-final-0 -----------
Name Stmts Miss Cover Missing
------------------------------------------------------------------------------------------
:
commitizen/wrap_stdio_unix.py 42 42 0% 1-73
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.
Got it. From my point of view, we should write cleaner code instead of just trying to match the check. I'm ok with skipping part of the coverage check if it does make the code cleaner.
commitizen/wrap_stdio_unix.py
Outdated
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.
As this is not used, please remove this line
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 will remove it.
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.
please remove the redundant blank line
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 will remove it.
Thank you for your good comment and sorry to my late reply.
next commit is bellow.
Wrap standard in/out files turn to the package.
cz commit will be walk well.
It seems the pipeline is due to lack of runner which is our side of the problem. I'll take a look at it these days and see how we can tackle it.
It seems the pipeline is due to lack of runner which is our side of the problem. I'll take a look at it these days and see how we can tackle it.
This PR #456 should address this issue
6112f8c to
14f4182
Compare
I just tried the latest version on my Mac but failed
Failed
- hook id: commitizen-prepare-commit-msg
- exit code: 1
also, we'll need to address #250 (review)
Hi, @Lee-W san.
Sorry to this trouble.
I tell you about sad thing.
I am far from Mac, so I don't know about Mac OS specification well.
For this PR I bought used Mac mini(2014) and OS is 10.15.7.(new Mac is expensive)
This programming woks well my Mac.
I try to upgrade my Mac to latest OS, but hung and reboot.
So in this time I have no way to what is happen.
Would you mind tell me more information about error.
@saygox Sure! I was trying to dig a bit but didn't find out the cause. Will run a deeper investigation when I have time. Thanks!
add --commit-msg-file argument
This reverts commit f7f80bb.
1053d01 to
a7b3228
Compare
Https://github.com/commitizen-tools/commitizen/pull/731 suppressed this PR. close this one
Types of changes
Description
invoke cz commit from git prepare-commit-msg hook
Checklist
./script/formatand./script/testlocally to ensure this change passes linter check and testSteps to Test This Pull Request
Expected behavior
Additional context