I'm shocked how many companies still don't focus on DevX and wonder why not only is productivity in the gutter, but morale as well. There's so many companies that help solve DevX problems like this, that's it's practically easy to throw money at it.
PR process killing morale and productivity
31–40 of 224 posts
Re: PR process killing morale and productivity
#32Recently I allowed everyone in my team to push to develop without PRs and even without doing feature branches. We review the code all together (max 2 ppl) just before the release and that's it. But great CI pipeline is must for this. It will get even better soon when AI will be able to slightly refactor the code.
> We review the code all together (max 2 ppl) just before the release and that's it.
Is there a reason why you don't review the code immediately? You could also do that without PRs.
Re: PR process killing morale and productivity
#33Personally I have a strong distaste for projects that try to use some metric for how long a function should be, e.g. line count or cyclomatic complexity. I'm not sure if this one is better automated, human judgement for what makes sense seems better to me. Sometimes the cleanest and highest performance way to write some code is going to be basically one relatively large function. If there's a function complexity/size…
It shouldn't be a hard rule, but in most cases a sufficiently long function will have opportunities for pulling out functions with sensible signatures that make sense outside that context.
All a linter does automatically is require adding the "ignore this line" comments that are then visible to humans in diffs.
Re: PR process killing morale and productivity
#34200 comments mostly about stylistic nitpicking is actually a language fault, not a team fault.
Languages like Golang are specifically designed to remove any stylistic nitpicking.
Every language must follow. It's suicidal otherwise.
Re: PR process killing morale and productivity
#35My team has a bike shedding sort of problem where a 100 loc PR will sometimes get scrutinized to hell, but a 3,000 loc PR will get LGTM'd by enough of the team to be merged before anyone that actually cares gets a chance to look at it. I would say the second half of that is the much bigger problem. People know who to ask to get a quick lgtm. I don't know what to do about it. I can't make people actually review. I've…
The reality is that you can't make people to care more about quality than their (yours) management cares about it.
I would suggest discuss this with manager, and if he cares enough about quality, he can try to slowly coach the individuals to give better reviews. Or he can suggest that multiple experienced/reliable people also review the work.
Re: PR process killing morale and productivity
#36> I’ve recently come across a discussion where a new developer joined a team and faced over 300 PR comments on their first contribution. Most of it was stylistic nitpicking. This isn’t just unproductive, it’s outright toxic. For me this says more about the company culture than any inherent flaws with the code review process.
IMO the best way to handle stylistic choices (if at all) is to have the reviewer just adjust it themselves. Ping-pong with comments like "add empty line" is a waste of time for everyone.
I like this idea, it has the bonus of revealing how much the reviewer actually cares. I bet half the things they wouldn’t take the time to fix themselves.
Re: PR process killing morale and productivity
#37Another angle is that a PR with over 300 comments is probably way too big. Many small focused PRs is IMAO much better than big PRs with weeks or months of work in them. But that requires fast and friendly PR reviews!
It also requires good tooling and some experience.
GitHub absolutely sucks at making multi-commit features. At least last time I used it.
And getting people to actually make smaller commits and reviews is incredibly hard.
Re: PR process killing morale and productivity
#38> I’ve recently come across a discussion where a new developer joined a team and faced over 300 PR comments on their first contribution. Most of it was stylistic nitpicking. This isn’t just unproductive, it’s outright toxic. For me this says more about the company culture than any inherent flaws with the code review process.
Honestly for me it tells me they need to hook up a code auto formatter into their workflow Forget stylistic nitpicking. Enforce a code quality standard with a linter and formatter and be done with it
Automating this with linter and formatter is great. It moves the argument over style and format to a one liner change to a lint config instead of mingling it with the with the main code change.
Re: PR process killing morale and productivity
#39> I’ve recently come across a discussion where a new developer joined a team and faced over 300 PR comments on their first contribution. Most of it was stylistic nitpicking. This isn’t just unproductive, it’s outright toxic. For me this says more about the company culture than any inherent flaws with the code review process.
This kind of feedback has largely been solved, for me / my team at least, with linters and formatters (as mentioned in the article). So I'd say it is still a reflection on the code review process being broken.
Re: PR process killing morale and productivity
#40My team has a bike shedding sort of problem where a 100 loc PR will sometimes get scrutinized to hell, but a 3,000 loc PR will get LGTM'd by enough of the team to be merged before anyone that actually cares gets a chance to look at it. I would say the second half of that is the much bigger problem. People know who to ask to get a quick lgtm. I don't know what to do about it. I can't make people actually review. I've…
Have you tried the code owners feature (assuming you're on Github). IMO a good approach is to have the actual code owners (i.e. the team responsible for a specific service or library) review the PR. If they think a shallow LGTM review of 3k LOC is enough, they can also deal with the bugs :-) If you don't have specific ownership in your code base I'd start there.
This is all within a relatively small single team.
As I said however, I could require review from specific people I know review but they're already at their wits end.
Also having to explain why certain devs are required without it smelling like some sort of favoritism seems fraught.