Hacker News Reader

That's not a code review. That's either a suggestion that the initial commit is 'wrong' or the reviewer is a jerk. Either way, this is the PM's fault - not enough information to finish it right or too reviewer nitpicky on useless stuff.

View on HN

There's a really good code review guide on the plaid blog that addressed this: https://blog.plaid.com/building-an-inclusive-code-review-cul....

Money quote: "Nothing should be surprising to a reviewer in a code review: discuss any significant design decisions before code is written. Code reviews are a time to iron out implementation details, not discuss major design or architecture decisions."

What if you use pull requests to prototype architectural change? My take is your pull request should include the context of the change and the sort of feedback you want.

I think that explicitly noting “this isn’t ready, but I’d like to discuss x,y,z” ought to cover it.

It was pretty common at my last job; seemed like every third PR got at least one request for a total or near-total rewrite, usually for completely non-functional changes without obvious benefits over the solution implemented.

I've been asked to make my solution more complicated and to add usage of things like the friend keyword to unit tests instead of testing a public static function.

Reply on news.ycombinator.com