Skip to content

Add a verbose hook option #695

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.

Sign up for GitHub

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
asottile merged 1 commit into from
Feb 1, 2018
Merged

Add a verbose hook option #695

asottile merged 1 commit into from
Feb 1, 2018

Conversation

@bagerard
Copy link
Contributor

@bagerard bagerard commented Jan 31, 2018

Relates to #689
It adds a verbose flag that can be set per hook and that allows stdout to be printed even when hook returned 0

asottile
asottile approved these changes Jan 31, 2018
View reviewed changes
Copy link
Member

@asottile asottile left a comment

Choose a reason for hiding this comment

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

Looks good! I think only the first test is necessary -- let's remove the other two (they're well written 👏 -- but not necessary)

tests/commands/run_test.py
(b'Hello World',),
0,
stage=False,
)
Copy link
Member

@asottile asottile Jan 31, 2018

Choose a reason for hiding this comment

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

this test is probably sufficient, the others I don't think are super necessary (we already test all three of those scenarios)

@asottile
Copy link
Member

asottile commented Jan 31, 2018

Also looks like there's a test that needs 'verbose': False here:

pre-commit/tests/repository_test.py

Lines 791 to 792 in 7f0b427

'exclude_types': [],
}

bagerard pushed a commit to bagerard/pre-commit that referenced this pull request Jan 31, 2018
@cp-bagerard
Add a hook option that allows stdout to be printed when exit code is 0 (
38eff71 
pre-commit#695)
@bagerard
Copy link
Contributor Author

bagerard commented Jan 31, 2018

Thanks @asottile for the immediate review. Should I do a PR to document the flag in pre-commit.github.io ?

@asottile
Copy link
Member

asottile commented Jan 31, 2018

@bagerard that would be awesome :) source is https://github.com/pre-commit/pre-commit.github.io (branch from real_master)

@asottile
Copy link
Member

asottile commented Jan 31, 2018

Oops, one small comma needed :) (pre-commit && pre-commit run --all-files will put it there for ya)

@bagerard
Copy link
Contributor Author

bagerard commented Feb 1, 2018

Indeed I should have thought to run pre-commit on pre-commit :) Here you go

@asottile asottile merged commit 17444cd into pre-commit:master Feb 1, 2018
@asottile
Copy link
Member

asottile commented Feb 1, 2018

Thanks again!

@asottile asottile mentioned this pull request Feb 1, 2018
Closed
@chadrik chadrik mentioned this pull request Apr 21, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@asottile asottile asottile approved these changes

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@bagerard @asottile @cp-bagerard