I had a new dev that was tasked with changing the colors of a button. took them 3 days to do it.
when I finally got the PR there were over 300 files changed and over 100k changes.
they "fixed" the while space. I asked them who asked them to do that. "Nobody, it's just maintenance on the codebase."
I then had an hour long call with them reviewing everywhere they "fixed" the whitespace and got through 10% of it. I asked them if they wanted to have another call tomorrow to review the rest or if they wanted to put the changes forward they were tasked with.
I mean, that could be extreme, or really not that bad.
Refactors have a way of generating a lot of changes. Half our job is code review, kind of have to get over it and go read some code.
If someone put the effort in to write it, it's your responsibility to put the effort in to read it and review it.
If the style is difficult to read and non-standard for your repository or not. Conventional then your repository and your engineering team should be following set standards to ensure consistency.
If you're doing this then most PRS shouldn't be that difficult to review.
I say this, spending a decent part of my week reviewing something like 40+ PRs.
i have a recent diff where 300 changes are just things like moving whitespace
still fuck you for that mess
I have vim setup to trim trailing white space, I get so pissed when a small change results in a 300 line diff because of poor code hygiene.
Do ya’ll just not use linters?
https://github.com/axelf4/vim-strip-trailing-whitespace
There are plugins that will only trim modified lines, which resolves that issue
I had a new dev that was tasked with changing the colors of a button. took them 3 days to do it.
when I finally got the PR there were over 300 files changed and over 100k changes.
they "fixed" the while space. I asked them who asked them to do that. "Nobody, it's just maintenance on the codebase."
I then had an hour long call with them reviewing everywhere they "fixed" the whitespace and got through 10% of it. I asked them if they wanted to have another call tomorrow to review the rest or if they wanted to put the changes forward they were tasked with.
they got the hint and fixed their shit.
Does the work item describe this technical cleanup? No? PR rejected for including out of scope work.
please tell me you have an accepted code style with proper tooling and precommit hooks set up
this, this is what I am dealing with, thank you for teaching him