alt Hacker NewsThis is a hill I’m going to die on, but I find 9/10 times people use the pr description for what should have been comments. “Git blame” and following a link to a pr is inferior ux to source code comments.
The North Star of pr review is zero comment approvals. Comments should not be answered in line, but by pushing updates to the code. The next reader otherwise will have the exact same question and they won’t have the answer there.
The exception being comments which only make sense for the sod itself but not the new state of the code. IME that’s ~10%.
I have bought my tombstone.
Replies
> Comments should not be answered in line, but by pushing updates to the code.
Hear, hear.
me: This unreadable, needs a comment.
them: <explains the thing>
me: True, but I've meant a source code comment.
them: <explains the thing again, but with more words; push never happens>
But there are really two types of things you need to describe in a change request:
- What and why needs changing
- What the code does after the change
One should really try hard to keep the first one answered in a change request description, or comments in the tool for code reviews. Don't you love running into comments in the code of the type "// This performs better than sorting-after-load as the service offers built-in sorting." because someone originally did "load(); in_memory_sort()" and today the code only does a "load(order_by=X)" (I mean, duh).
The resulting code should only have comments that explain the why for the end-state code.
But yes, questions to explain something in the end-state should always trigger changes in the code: make code more self-explanatory!