Hacker News new | past | comments | ask | show | jobs | submit login

I don't know the original story, but 300 hundred PR comments on a first PR (or any PR for that matter) is insane, and my guess is that somebody on the team is having an argument back-and-forth in that PR, rather than focusing on whether the code should be merged.

If that happens to me, I would kindly invite them to talk about it elsewhere and give me pass, because the fact that you guys are still debating it, means that it's not a decided thing, and you can't prosecute me with a law that hasn't passed yet. Once it's actually settled, I'll be happy to go back and refactor it, but it is illogical to make it a blocker in the present.

It also sounds like something is wrong with the on-boarding process. The first PR should not be of such a nature that would invite loads of comments. It should be something that is functionally simple, and already has an agreed-upon design.






I was once on a team where this was not unusual, and the problem was the tech lead was the one engaging in 300 PR comment arguments.

This would result in stuff sitting in purgatory forever.

The funniest part was this was, in 20 years, the absolute worst codebase I'd ever seen, so he wasn't even maintaining some high standards in the codebase by doing so.


Sounds about right. If so many technical decisions are made by a single individual while reviewing PR, rather than having a proper standard agreed upon before hand, it's no wonder that it grows into a horrible code base.

I think PRs are also the wrong place to litigate any of this.

Requirements, specs, architecture, etc sure.

But most systems I've worked on that were especially BAD were that way not because of some PR-reviewable syntax or style, but because the whole foundation was wrong.

The most idiomatic, consistent, linted, unit-tested, etc code in the world doesn't matter that much if the underlying architecture is a mess.




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: