Skip to content

Support fail_fast on check level#2097

Merged
asottile merged 1 commit intopre-commit:masterfrom
colens3:master
Oct 22, 2021
Merged

Support fail_fast on check level#2097
asottile merged 1 commit intopre-commit:masterfrom
colens3:master

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

Choose a reason for hiding this comment

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

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

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

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

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 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 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

should be fixed on master -- please try rebasing

@colens3
Copy link
Contributor Author

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

Choose a reason for hiding this comment

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

@asottile asottile merged commit 663a766 into pre-commit:master Oct 22, 2021
@Anthchirp
Copy link

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

Development

Successfully merging this pull request may close these issues.

Support fail_fast on check level

3 participants