Hacker News Reader

Comment by colemorrison | Hacker News Reader

In the "How it all started section" -

> E explained that the main impetus behind the introduction of code review was to force developers to write code that other developers could understand; this was deemed important since code must act as a teacher for future developers.

This should always be at the forefront of the code reviewer's mindset. It's quite easy for the review process to degenerate into a style argument, quest to find every inefficiency, or attempt to make code as "clever" and brief as possible.

View on HN

I think a key to instilling positive code review culture is to make sure that the comments are based on some objective, such as readability and clarity, and not based on personal preference. In particular, if one person is reviewing code of another, it is not helpful for the reviewer to remark that they would have written it differently, if they had written it at all. Some people for some reason can't keep themselves from doing this. They should remember that they did not, in fact, write the change, so their personal style is irrelevant. If code is clear, obviously correct, well-tested, and correctly formatted, it LGTM.

except the definition of readability varies and we're back to square one

Yes well in the final analysis all problems are recruiting problems, aren’t they?

Unless companywide guidelines are agreed upon, and published.

E.g http://google.github.io/styleguide/javaguide.html

https://github.com/google/styleguide/blob/gh-pages/pyguide.m...

https://developers.google.com/edu/python/introduction: According to the official Python style guide (PEP 8), you should indent with 4 spaces. (Fun fact: Google's internal style guideline dictates indenting by 2 spaces!)

https://github.com/google/styleguide/blob/gh-pages/pyguide.m...: Indent your code blocks with 4 spaces.

Not really.

Between serious professionals, there will be a few disagreements, but (1) you can work through them with some give and take, and (2) over time you get to know each other, which usually means one persons agrees the other's style is better, or you agree to to disagree on on point.

I think putting opinions in code review comments is fine, as long as you’re clearly indicating it as an opinion. The worst is when a reviewer tries to justify their opinionated comment with some vague appeal to style or readability instead of “I hate this part, ship it”

Reading another human's code (or prose) is the closest thing we have to mind reading. Whenever I can't avoid doing code reviews, most often, I really wish I didn't know that much about the author.

"...code must act as a teacher..."

Agree. Alas, most teachers aren't very good.

https://en.wikipedia.org/wiki/Sturgeon's_law

I used to really care about this stuff. I started a design pattern study group 25 (?) years ago. It's still going strong. (Geeks love to eat pizza and argue about the rules. :) )

Professionally, I've all but given up. Any more, I just try to mimic the code style of whatever goo I'm working on. And do whatever kabuki required of me to get my PRs merged.

Programming is now my hobby. Whenever I feel the need to code with intent and feeling, I work on a personal project. (For some reason, this reminds me of Al Pacino quote about exercising. Something like "I just lie down until the feeling goes away." Happily, I'm not that bleak.)

When I am feeling optimistic, I imagine a future where we adopt a hybrid of Pieter Hintjens' Social Architecture and Michael Bryzek's Test in Production.

https://legacy.gitbook.com/book/hintjens/social-architecture...

https://qconsf.com/sf2017/sf2017/presentation/testing-produc...

TL;DR: Accept all PRs, radically reduce the cost of changes.

I think I mostly accept anything that comes my way, except for ones that have obvious, non-blocking fixes.

People tell me to be more critical, but there’s no point in holding everything up because someone wrote a triple nested ternary operator.

It’s icky, but they, or someone else, will fix it the next time they come across that piece of code. That’s why we have tests.

Reply on news.ycombinator.com