| ▲ | phito 4 days ago |
| I often write PR descriptions, in which I write a short explanation and try to anticipate some comments I might get. Well, every time I do, I will still get those exact comments because nobody bothers reading the description. Not to say you shouldn't write descriptions, I will keep doing it because it's my job. But a lot of people just don't care enough or are too distracted to read them. |
|
| ▲ | simonw 4 days ago | parent | next [-] |
| For many of my PR and issue comments the intended audience is myself. I find them useful even a few days later, and they become invaluable months or years later when I'm trying to understand why the code is how it is. |
|
| ▲ | ffsm8 4 days ago | parent | prev | next [-] |
| After I accepted that, I then tried to preempt the comment by just commenting myself on the function/class etc that I thought might need some elaboration... Well, I'm sure you can guess what happened after that - within the same file even |
|
| ▲ | walthamstow 4 days ago | parent | prev | next [-] |
| At my place nobody reads my descriptions because nobody writes them so they assume there isn't one! |
| |
|
| ▲ | nothrabannosir 3 days ago | parent | prev | next [-] |
| This 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. |
| |
| ▲ | necovek 3 days ago | parent | next [-] | | 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! | | |
| ▲ | nothrabannosir 3 days ago | parent [-] | | Yes totally agree, I now see a crucial auto correct typo in my original comment wherei was trying to say the same but failed xD. “sod” = “diff”. 100% agree with your comment. |
| |
| ▲ | kubanczyk 3 days ago | parent | prev [-] | | > 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> |
|
|
| ▲ | skydhash 4 days ago | parent | prev | next [-] |
| I just point people to the description. no need to type things twice. |
| |
| ▲ | lanstin 4 days ago | parent | next [-] | | Sadly, when communicating with people, important things have to be repeated over and over. Maybe less so with highly trained and experienced people on something that their training and experience make the statement plausible, but if the thing is at all surprising or diverges from common experience, I've found a need to bang it out via multiple communication channels. | | |
| ▲ | simonw 4 days ago | parent [-] | | I learned this lesson as an engineering manager / tech lead. I got frustrated at how often I found myself having the exact same conversation with different people... until I relapsed that communicating the same core information to different people was a big chunk of the job! |
| |
| ▲ | wiml 4 days ago | parent | prev [-] | | "I think I covered that in the (PR text | comment two lines up | commit message), did you have an issue I didn't address there?" Maybe that's the AI agent I would actually use, auto-fill those responses... |
|
|
| ▲ | necovek 3 days ago | parent | prev | next [-] |
| What is the overall practice in the team? Does everybody write good descriptions and nobody reads them, or only a few of them write good descriptions and nobody reads them? Because if it's the latter, there's your problem: even those who write good descriptions do not expect a change request to have one, so they don't bother looking. |
|
| ▲ | JohnBooty 3 days ago | parent | prev [-] |
| Yeah, I've definitely found that nobody reads more than maybe 10 words of the PR description. I've also never seen anybody but myself write substantial PR descriptions at my previous 4-5 jobs |
| |
| ▲ | necovek 3 days ago | parent [-] | | But if nobody writes them, they don't have the habit of reading them either. However, also make sure your PR descriptions are not "substantial" in the "there is a lot of it" sense, but only "substantial" in the "everything of substance is described, but not more" sense :) |
|
