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

Support fail_fast on check level #2097

Merged
merged 1 commit into from Oct 22, 2021
Merged

Support fail_fast on check level #2097

merged 1 commit into from Oct 22, 2021

Conversation

@colens3
Copy link
Contributor

@colens3 colens3 commented Oct 19, 2021

This PR adds support for fail_fast on per-hook level. Additional test created to cover new use case, in addition to that the old test was updated due to the change in Hook class (added fail_fast attribute).

Closes #1143

Copy link
Member

@asottile asottile left a comment

looks good! just one small thing

also a tip for the future, it's usually best to do feature development on a separate branch from master that way it's easier for me to help edit code if needed

@@ -78,6 +78,7 @@ def _make_argparser(filenames_help: str) -> argparse.ArgumentParser:
cfgv.Optional('require_serial', cfgv.check_bool, False),
cfgv.Optional('stages', cfgv.check_array(cfgv.check_one_of(C.STAGES)), []),
cfgv.Optional('verbose', cfgv.check_bool, False),
cfgv.Optional('fail_fast', cfgv.check_bool, False),
Copy link
Member

@asottile asottile Oct 19, 2021

Choose a reason for hiding this comment

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

can you put this up next to always_run (to group related attributes)

Copy link
Contributor Author

@colens3 colens3 Oct 19, 2021

Choose a reason for hiding this comment

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

Sure thing!

@colens3
Copy link
Contributor Author

@colens3 colens3 commented Oct 19, 2021

looks good! just one small thing

also a tip for the future, it's usually best to do feature development on a separate branch from master that way it's easier for me to help edit code if needed

Sure! Tnx for the tip

@colens3
Copy link
Contributor Author

@colens3 colens3 commented Oct 19, 2021

@asottile Noticed that two jobs have failed but not sure how can I resolve those issues, please let me know if there is something I can do

@asottile
Copy link
Member

@asottile asottile commented Oct 20, 2021

should be fixed on master -- please try rebasing

@colens3
Copy link
Contributor Author

@colens3 colens3 commented Oct 20, 2021

should be fixed on master -- please try rebasing

That resolved the issue!

Copy link
Member

@asottile asottile left a comment

@asottile asottile merged commit 663a766 into pre-commit:master Oct 22, 2021
1 of 2 checks passed
@Anthchirp
Copy link

@Anthchirp Anthchirp commented Jan 10, 2022

I have now started using this, and it works exactly as you'd expect. Thank you ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants