Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

PR process killing morale and productivity

blackentropy.com

Post reply on HN
Print view View original

101–110 of 224 posts

Re: PR process killing morale and productivity

#101

Post by tremon »

rileymat2 wrote: post #68

Part of me wonders if the real issue is we don't have an author/editor type system. If the reviewer/editor could just make the nitpicky changes in about the same time it takes to call them out, the relationship might be much more healthy. Things could go back and forth in a much more healthy way.

We do, in a way, but it's not recognized as such. The "author" would be the mythical 10x developer that races to get feature tickets completed, the editors are the ones getting stuck with integrating a pile of stinking hotness into the existing codebase. That kind of setup can work fine if everyone is aware of the others' value, but usually non-technical managers only see the hotness.

Re: PR process killing morale and productivity

#102

Post by fmbb »

dec0dedab0de wrote: post #21
znpy wrote: post #9

Earlier quoted context omitted.

on a different side: it also tell you (a lot) about specific people. I've seen good/great people call out the nitpicks (in my case it was often mis-spelling, due to not being a native speaker of english) but will approve the PR anyway (implicitly expecting another revision to be sent, trusting the submitter). On the other hand bad/toxic people will drown you with stylistic nitpicks and won't approve (and trust) you t…

* I've seen good/great people call out the nitpicks (in my case it was often mis-spelling, due to not being a native speaker of english) but will approve the PR anyway (implicitly expecting another revision to be sent, trusting the submitter).* I always thought this was the best way. I wish these systems had a way to assign severity to comments, and urgency to the commit. If you have a jr developer it is your job to…

I like https://blog.danlew.net/2020/04/15/the-logaf-scale/

Re: PR process killing morale and productivity

#104

Post by jibbit »

People being an obstacle to positive change, because they believe a better change is possible.. when the changes wouldn't in any way prevent further positive changes from happening.. are the bane of my life. 'bikeshedding' probably covers it, but i feel like if someone could come up with a better metaphor that wasn't specifically about triviality or level of comprehension it could be a profound contribution to human behaviour

Re: PR process killing morale and productivity

#105

Post by mdaniel »

PhilipRoman wrote: post #29
barbazoo wrote: post #5

> 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 GitLab's feature of "suggested change" whereby the submitter can offer the change as a one-click "accept change" but importantly the change is still under the original author, because in any sane(?) review process, authors are not able to approve their own merge request. In your workflow if the reviewer checked out the branch, did the needful, committed, and then resumed reviewing they'd have locked themselves out of approving (err, assuming the aforementioned "authors cannot approve their own merges")

https://docs.gitlab.com/ee/user/project/merge_requests/revie...

https://docs.gitlab.com/ee/user/project/merge_requests/appro... and https://docs.gitlab.com/ee/user/project/merge_requests/appro...

Re: PR process killing morale and productivity

#106

Post by tired_and_awake »

If you join a company and are confronted with dozens of PR comments - assuming this process isn't sufficiently well documented - see if a tech lead or manager will host an open forum review. Review the code and the comments and discuss + document. A one off discussion won't be a permanent fix, but it can help!

Re: PR process killing morale and productivity

#107

Post by mihaaly »

OptionOfT wrote: post #80
barbazoo wrote: post #5

> 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 is why I love things like cargo fmt / go fmt / eslint / etc. No discussions of whether if (foo) return; is valid, or we should do if (foo) { return; }

Who cares?!

Re: PR process killing morale and productivity

#108

Post by MortyWaves »

dukedylan wrote: post #31

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.

I ask myself this often, usually while despairing that the lead dev doesn’t see any point to build pipelines, unit tests, SQL migrations (literally right clicks and edits a database schema), and the requirement of needing Remote Desktop to a VM to do anything.

Re: PR process killing morale and productivity

#109

Post by loa_in_ »

lijok wrote: post #63
donatj wrote: post #23

My 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…

Speaks volumes to the maturity of our work that a 3000 LOC PR is seen not only as acceptable, but expected. Raising a planning change equivalent to a 3000 LOC PR in a civil engineering firm would get your assignment swiftly handed over to someone more competent.

LOC is a bad measure of literally anything. It's not related to productivity, efficiency, bug count, readability, etc. What it's most related to is the choice of language and libraries. For me, shunning big contributions just for being big just stifles my drive to contribute. If the solution does what it's supposed to, is valid, efficient and there's no obvious unnecessary duplication of codethen who cares if it's big. All valid contributions push the project forward.

Re: PR process killing morale and productivity

#110

Post by onion2k »

barbazoo wrote: post #5

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

It says PR comments benefit the reviewer in some way, and they're not always left to improve the code or to help the person who opened the PR. For example, if you work in a culture where being seen to leave PR comments is seen as positive regardless of the quality of them, or even that code review is a tracked metric that contributes to your performance review.

That culture will turn even the most conscientious dev into a monster.

Post reply on HN