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

Feature request : add a "fail fast" configuration entry #240

Closed
Lucas-C opened this issue Jun 3, 2015 · 8 comments
Closed

Feature request : add a "fail fast" configuration entry #240

Lucas-C opened this issue Jun 3, 2015 · 8 comments
Labels

Comments

@Lucas-C
Copy link
Contributor

Lucas-C commented Jun 3, 2015

Here is my use case:

  • I have integration tests (in Java + with protractor) that are costly to execute (in term of execution time)
  • when earlier hooks detect issues with the code (e.g. eslint checks), I don't want to execute my integration tests until I've fixed those errors

Btw, I don't ask to change the current behaviour, but could you develop on the reasons behind the design decision to run all tests, indifferently of their outcome ? What are the benefits ?

@asottile
Copy link
Member

asottile commented Jun 3, 2015

The main benefit to running all of the hooks is for fixers. That said, this seems like a reasonable idea. The unfortunate bit is there isn't a good place to put it in .pre-commit-config.yaml without breaking the schema :S

@Lucas-C
Copy link
Contributor Author

Lucas-C commented Jun 4, 2015

Agreed.
I can't find the relevant issue but I remember seeing a plan to get rid of the JSON schemas : is that still considered ? And then, should this be pending on json schema removal ?
Alternatively we could maybe simply extend the schema to allow this optional new field.
It won't "break" anything in the sense that it will still be backward compatible.

@asottile
Copy link
Member

asottile commented Jun 4, 2015

Oh I imagined we would want a top level all-or-nothing option instead of having to repeat it in every hook. The breaking change I was thinking of was changing the config to be a map instead of a list (Which would enable a bunch of other things I've wanted to do). Apis are hard.

And yeah I'm planning on ditching JSON schema. if we're just adding a property to hooks it's probably not breaking (we've done it in the past)

@Lucas-C
Copy link
Contributor Author

Lucas-C commented Jun 4, 2015

Ok, I imagined on my part that we could have something like this:

- config:
  - fail-fast: false
- repo: local
  hooks:
  - id: do_not_commit
    name: Block if "DO NOT COMMIT" is found
    entry: DO NOT COMMIT
    language: pcre
    files: ''
- config:
  - timeout: 120s
  - fail-fast: true

(The repetition of the same config property is just here to show a possible edge case, and because I lack imagination :D)

@asottile
Copy link
Member

asottile commented Jun 6, 2015

I'd rather have a format that looks like this, though migration will be somewhat difficult:

fail-fast: true
repos:
-   repo: local
    hooks:
    -   id: ...
        ...

@asottile
Copy link
Member

asottile commented Sep 8, 2017

This will be available in 1.1.0 (once released) -- I plan to also throw in a couple other things as well into this release.

The format for failing fast is:

fail_fast: true
repos:
- ...

as such it is only available with the v2 configuration introduced in 1.0.0

@Lucas-C
Copy link
Contributor Author

Lucas-C commented Sep 11, 2017

Awesome, thanks !

@asottile
Copy link
Member

This is now available in v1.1.0! Thanks again for the issue!

droctothorpe pushed a commit to droctothorpe/pre-commit that referenced this issue Mar 23, 2022
…orce

Fix mixed-line-endings --fix=... when whole file is a different ending
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

2 participants