feat(bump): add optional --no-verify argument for bump command #183

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

Jump to bottom

Merged

Lee-W merged 3 commits into commitizen-tools:master from kudlatyamroth:bugfix/fix-bumping-with-pre-commit

May 5, 2020

Merged

feat(bump): add optional --no-verify argument for bump command #183

Lee-W merged 3 commits into commitizen-tools:master from kudlatyamroth:bugfix/fix-bumping-with-pre-commit

May 5, 2020

Conversation

kudlatyamroth

Copy link

Contributor

@kudlatyamroth kudlatyamroth commented May 4, 2020 •

edited

Loading

fix: #164

When we have in repo some pre-commit hooks, like black, mypy or anything like that
cz bump fails, even if pre-commit hooks passed.

it is strange as pre-commit write messages to sys.stdout.buffer not stderr...

example pre-commit config:

repos:
 - repo: local
 hooks:
 - id: flake8
 name: flake8
 stages: [commit]
 language: system
 entry: poetry run flake8
 types: [python]
 exclude: setup.py
 - id: mypy
 name: mypy
 stages: [commit]
 language: system
 entry: poetry run mypy
 types: [python]
 pass_filenames: false

This PR add optional --no-verify argument to bump command

@codecov

Copy link

codecov bot commented May 4, 2020 •

edited

Loading

Codecov Report

Merging #183 into master will increase coverage by 0.26%.
The diff coverage is 100.00%.

Impacted file tree graph

@@ Coverage Diff @@
## master #183 +/- ##
==========================================
+ Coverage 91.76% 92.02% +0.26% 
==========================================
 Files 35 35 
 Lines 947 953 +6 
==========================================
+ Hits 869 877 +8 
+ Misses 78 76 -2

Flag	Coverage Δ
#unittests	`92.02% <100.00%> (+0.26%)`	⬆️

Impacted Files	Coverage Δ
commitizen/cli.py	`91.17% <ø> (ø)`
commitizen/__version__.py	`100.00% <100.00%> (ø)`
commitizen/commands/bump.py	`88.37% <100.00%> (+3.37%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 86f67fd...638b38a. Read the comment docs.

@kudlatyamroth

Copy link

Contributor Author

kudlatyamroth commented May 4, 2020

@woile what do you think about that?
personally i think its hacky way to fix this, but can't find out why stdout from pre-commit is taken as stderr...

@Lee-W

Copy link

Member

Lee-W commented May 5, 2020

I also encounter the same problem. I'm thinking of adding a --no-verify argument to cz bump command to make it optional for users.

@kudlatyamroth

Copy link

Contributor Author

kudlatyamroth commented May 5, 2020

@Lee-W i changed this pr to make --no-verify as optional argument :)

@kudlatyamroth kudlatyamroth force-pushed the bugfix/fix-bumping-with-pre-commit branch from 846a31f to 4b79ab1 Compare

May 5, 2020 05:47

@kudlatyamroth kudlatyamroth changed the title ~~(削除) fix(bump/pre-commit): pass --no-verify to bump commit (削除ここまで)~~ (追記) feat(bump): add optional --no-verify argument for bump command (追記ここまで)

May 5, 2020

@kudlatyamroth kudlatyamroth force-pushed the bugfix/fix-bumping-with-pre-commit branch from 4b79ab1 to 6816c6e Compare

May 5, 2020 05:50

Lee-W

Lee-W reviewed

May 5, 2020

View reviewed changes

Copy link

Member

@Lee-W Lee-W left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @kudlatyamroth ! This problem bothers me for some time. This PR looks great! But you might need to add some test cases to cover this new argument so that we'll be able to merge it.

commitizen/cli.py Outdated

"name": ["--no-verify"],

"action": "store_true",

"default": False,

"help": "disable git hooks during bumping",

Copy link

Member

@Lee-W Lee-W May 5, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just take a look of the documentation of git. The help message is

--no-verify
 This option bypasses the pre-commit and commit-msg hooks.

I think it might be better if we can align with them

@kudlatyamroth kudlatyamroth force-pushed the bugfix/fix-bumping-with-pre-commit branch from 6816c6e to 5fc1033 Compare

May 5, 2020 12:33

kudlatyamroth added 2 commits

May 5, 2020 14:35

@kudlatyamroth


 feat(bump): add optional --no-verify argument for bump command

8dd079e

@kudlatyamroth


 docs(bump): add info about --no-verify option

b8f2894

@kudlatyamroth kudlatyamroth force-pushed the bugfix/fix-bumping-with-pre-commit branch from 5fc1033 to b8f2894 Compare

May 5, 2020 12:35

@kudlatyamroth


 test(bump): add test for --no-verify option

638b38a

@kudlatyamroth

Copy link

Contributor Author

kudlatyamroth commented May 5, 2020

@Lee-W @woile
description changed, and test added :)
also updated with master :)

Lee-W

Lee-W approved these changes

May 5, 2020

View reviewed changes

Copy link

Member

@Lee-W Lee-W left a comment •

edited

Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! @kudlatyamroth Thanks for your help! Let's merge it!

@Lee-W Lee-W merged commit 7ae1850 into commitizen-tools:master

May 5, 2020

@kudlatyamroth

Copy link

Contributor Author

kudlatyamroth commented May 5, 2020

@Lee-W thanks for merge :)
but there is no release. propably this line:
https://github.com/commitizen-tools/commitizen/actions/runs/96287380/workflow#L10

should check for bump: or bump: version

@Lee-W

Copy link

Member

Lee-W commented May 5, 2020

@kudlatyamroth It's probably due to #157 I'll take a look tomorrow

Labels

None yet

2 participants

@kudlatyamroth @Lee-W

Uh oh!

feat(bump): add optional --no-verify argument for bump command #183

feat(bump): add optional --no-verify argument for bump command #183

Uh oh!

Conversation

@kudlatyamroth kudlatyamroth commented May 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented May 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

kudlatyamroth commented May 4, 2020

Uh oh!

Lee-W commented May 5, 2020

Uh oh!

kudlatyamroth commented May 5, 2020

Uh oh!

@Lee-W Lee-W left a comment

Choose a reason for hiding this comment

Uh oh!

@Lee-W Lee-W May 5, 2020

Choose a reason for hiding this comment

Uh oh!

kudlatyamroth commented May 5, 2020

Uh oh!

@Lee-W Lee-W left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kudlatyamroth commented May 5, 2020

Uh oh!

Lee-W commented May 5, 2020

Uh oh!

Uh oh!

@kudlatyamroth kudlatyamroth commented May 4, 2020 •

edited

Loading

codecov bot commented May 4, 2020 •

edited

Loading

@Lee-W Lee-W left a comment •

edited

Loading