search

pre-commit-ci / issues

public issues for https://pre-commit.ci
17 stars 4 forks source link

When running hooks that fix files for you, would we really want the CI to be marked as failed? #18

Closed bastienboutonnet closed 3 years ago

bastienboutonnet commented 3 years ago

Been trying pre-commit.ci, thanks for the preview! (I'm frontleftbythespeaker on your twitch stream btw --anyway off to the more serious stuff).

I ran into a few times where I intentionally broke some black formatting on locally to get the precomit hook to fire on CI. Black step failed, files were fixed but then my CI still showed a ❌ .

I was wondering if it might make sense in those cases to do one or more of the following:

Or maybe this is absolutely mega totally violating some basic assumptions and opinions and in that case happy to hear what you have to say.

It could also be that I'm missing crucial pieces of documentation about pre-commit and in that case sorry about this, I'll go and do my research.

bastienboutonnet commented 3 years ago

Hmm it seems maybe I was mistaken or stuck in a loop of black and isort keeping on fixing each other.

But at least on this PR (https://github.com/bastienboutonnet/sheetwork/pull/244) where I added prettier it failed and then fixed and then ran again and made the CI green.

So I guess maybe this issue is closable 🤦

asottile commented 3 years ago

the commit on which things were fixed is incorrect and so it should be marked as a failure. when fixes are applied they also don't always fix the actual issue

only when a run is green should it be successful

bastienboutonnet commented 3 years ago

Yep all clear. Sorry about the false alarm. It makes total sense