> Trivial mistakes (formatting, unused deps, lint issues) should be fixed automatically, not cause failures.
Do people really consider this best practice? I disagree. I absolutely don't want CI touching my code. I don't want to have to remember to rebase on top of whatever CI may or may not have done to my code. Not all linters are auto-fixable so anyway some of the time I would need to fix it from my laptop. If it's a trivial check it should run as a pre-commit hook anyway. What's next, CI should run an LLM to auto-fix failing test cases?
Do people actually prefer CI auto-fixing anything?
I think this is where things went off the rails for him. Commiting back to the same branch that is running CI has too many gotchas in any CI system. You touched on the first issue, the remote branch immediately deviates unexpectedly from the local branch. Care has to be taken not to trigger additional CI runs from that commit.
Doing it in CI sounds like making things more complicated by resetting to remote branches after pushing commits. And, in the worst case, something that actually brakes code that works locally.
I have team members who complain that installing and running pre-commit is too much overhead, so instead I see them pushing commit after broken commit that tie up CI resources to fail on the pre-commit workflow. :(
I’ve had people like this. I’m with the other commenter that: Why do they have a say in this? No way I’m letting them decide each day when to format, what style to format to... Meet, discuss, pick a style, enforce formatting, screw you if you don’t follow.
I’m also with the other commenter about settings these things at the Editor level, but also at the pre-push level.
We benchmark how long it takes to format/lint only changed files, usually no more than a second, maybe two, but I admit for some languages this may take more. An editor with a language server properly setup would have helped you find issues earlier.
We also have reports for our CI pipeline linters, so if we see more than 1 report there, we sent a message to the team: It means someone didn’t setup their editors nor their git hooks.
If the checks take more than a second, yeah, probably pre-commit is not the place/moment. Reliability is important, but so is user experience. I had companies where they ran the unit test suite at the pre-commit level, alright? And that is NOT fine. While it sounds like it’ll find issues earlier, it’ll screw your developer time if they have to wait seconds/minutes each time they fix a comma.
Because at the institutional level, there isn’t the appropriate will to mandate that devs fix their local environments, and I don’t feel like burning my own political capital on that particular fight.
I'm one of these; I'm loathe to put anything between me and making a commit and most of our linters take several dozen seconds to run. That's unacceptable UX to me; I can disable with `--no-check`, but it's always annoying to remember that when the thing I most want to do is save my working state.
I'd rather have linting pushed into the editing process, within my IDE/VS Code/vim plugins, whathaveyou, where it can feedback-loop with my actual writing process and not just be some ancillary command I run with lots of output I never read.
Oh, yeah, ours aren’t nearly that bad. Our pre-commit checks are <1s and our pre-push are <10s. (And the worst-performing pre-commit hook is Git LFS that runs through pre-commit... maybe there’s some way to improve the performance there.)
We have a lot of IDE checks, but they’re just warnings when debugging (because devs complained, IMO reasonably, that having them as errors during dev was too inconvenient during development/debugging). CI fails with any warnings, and we have devs who don’t bother to check their IDE warnings before committing and pushing to a PR.
That part immediately made me short circuit out of the piece. That sounds like a recipe for disaster and an unnecessary complexity that just brings loads of new failure modes. Not a best practice.
Trivial mistakes in PRs are almost always signs of larger errors.
I'm new to CI auto-fixes. My early experience with it is mixed. I find it annoying that it touches my code at all, but it does sometimes allow a PR to get further through the CI system to produce more useful feedback later on. And then a lot of the time I end up force-pushing a branch that is revised in other ways, in which case I fold in whatever the CI auto-fix did, either by squashing it in or by applying it in some other way.
(Most of the time, the auto-fix is just running "cargo fmt".)
Do people really consider this best practice? I disagree. I absolutely don't want CI touching my code. I don't want to have to remember to rebase on top of whatever CI may or may not have done to my code. Not all linters are auto-fixable so anyway some of the time I would need to fix it from my laptop. If it's a trivial check it should run as a pre-commit hook anyway. What's next, CI should run an LLM to auto-fix failing test cases?
Do people actually prefer CI auto-fixing anything?