I am definitely guilt for that, but I find this approach really productive. We use small bug fixes as an opportunity to improve the code quality. Bigger PRs often introduce new features and take a lot of time, you know the other person is tired and needs to move on, so we focus on the bigger picture, requesting changes only if there is a bug or an important structural issue.
I always try to review the code anyway. There’s no guarantee that what they wrote is doing what you want it to do. Sometimes I find the person was told to do something and didn’t realize it actually needs to do Y and not just X, or visa versa.
I like to shoot for the middle ground: skim for key functions and check those, run code locally to see if it does roughly what I think it should do and if it does merge it into dev and see what breaks.
Small PRs get nitpicked to death since they’re almost certainly around more important code
Especially when you see a change in code, but not in tests ☠️
So you’re always behind, patching up small bits of code that don’t comply with your guidelines, while letting big changes with, by deduction, worse code quality through?
This and bike shedding.
Reviewing large PR’s is hard. Breaking apart large PR’s that are all related changes into smaller PR’s is also hard.
If I submit a big one, I usually leave notes in the description explaining where the “core” changes are and what they are trying to accomplish. The goal being to give the reviewers a good starting point.
I also like to unit test the shit out of my code which helps a lot. The main issue there is getting management to embrace unit tests. Unit tests often double the effort up front but save tons of time in the long run. We’re going to spend the time one way or the other. Better to do it up front when it’s “cheaper” because charging it to the tech debt credit card racks up lots of expensive interest.
Yeah, if you don’t want the next dev (or your future self) to accidentally undo that corner case you fixed, better put a unit test on it.
I can’t believe we still have to justify writing unit tests to management in the year 2024
Ask him to do 500 lines and he will never look at it, making you wait forever
Meanwhile, ask a c-suite to do 500 lines, and they party until they overdose.
Just give them 10 lines at a time from the 500 lines one. Is this how micromanagement was born?
This is why I always rename all the variables in the project on each PR.