5 ms·

I do such things with pre-commit. Doing it in CI sounds like making things more complicated by resetting to remote branches after pushing commits. And, in the

by stared 2y ago

I do such things with pre-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.

Marsymars 2y ago
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. :(
- michpoch 2y ago
  > I have team members who complain that installing and running pre-commit is too much overhead Why do they have a say in this? This is up to tech leadership to set standards that need to be followed.
- tenacious_tuna 2y ago
  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.
  - Marsymars 2y ago
    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.
- anbotero 2y ago
  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.
  - Marsymars 2y ago
    > Why do they have a say in this? 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. Agreed on the performance comments.

by stared 2y ago

Marsymars 2y ago

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. :(

michpoch 2y ago
> I have team members who complain that installing and running pre-commit is too much overhead Why do they have a say in this? This is up to tech leadership to set standards that need to be followed.
tenacious_tuna 2y ago
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.
- Marsymars 2y ago
  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.
anbotero 2y ago
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.
- Marsymars 2y ago
  > Why do they have a say in this? 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. Agreed on the performance comments.