4 ms·
I haven't been at a place yet where pair programming is so common or frequent that code reviews are not necessary, so it's definitely a very useful skill. I als
by imrehg 3y ago
I haven't been at a place yet where pair programming is so common or frequent that code reviews are not necessary, so it's definitely a very useful skill. I also see that many people indeed take it as burden or hoops to jump through, rather than an inherently useful exercise, as the article outlines it.
Mentioned in the "What" section is the matching of the stated goal of the PR with the reality: so rare to find anyone who puts any effort into writing good PR messages (not to mention good commit messages, but that's for another time). Linking to tickets, explaining the rationale, most other context is just skipped because being assumed (or assumed to be communicated through some other channels). Less than ideal.
In general the "what you put in is what you get out" applies very well both for writing good changes and PRs as well as good reviews. Applying similar principles as the author listed, I have a lot of good feedback from people that the comments in reviews helped them improve and occasions for rare actionable feedback on technical know-how.
- fjfuvucucuc 3y agoJust flag for changes. "Purpose unclear, please update description as if you were a professional developer getting paid for this."
- diarrhea 3y agoI enjoy writing good PR messages. They border on too elaborate, with too much detail (better to err on that side though). They often aren't read. "TL;DR" is what I hear, without shame. Similar to how "math sucks!" in high school was popular. Reading/writing documentation? That's for nerds! Then later on confusion and questions, all easily solved if the PR (Jira ticket, etc...) had been read properly, or at all. It's very disheartening.
- erikpukinskis 3y agoI also write pretty detailed PR messages… mostly as comments on specific lines of the PR, either in the code itself if they’re worth keeping around. Or GitHub comments if they’re only of temporary relevance. But I’m not sad at all if they’re not read. It’s there if people want/need it. If they don’t then it’s easily skipped. I’m also not mad if someone asks me a question that could’ve been answered by reading a comment… different people have different ways of learning and I am happy to show up in whatever mode they find convenient. If a specific colleague is frequently missing info that was available in comments maybe that’s a growth opportunity for them to up their pace, and I might drop something in their suggestion box, but we’re all learning.
- Paul-Craft 3y ago> "TL;DR" is what I hear, without shame. Give the people what they want, then. Why not literally put a "TL;DR" at the top, like an executive summary, or something? The important thing is that all the gory details are present for those who want and need them. For those who may not, such as people not reviewing and approving the PR, a literal TL;DR can be a godsend.
- glenjamin 3y agoThe purpose of pair programming and the purpose of code review are not quite the same. Pair programming is a high-context activity where two programmers collaborate closely on building something. Code review is a tool to simulate looking at code that you haven't just worked on with high-context, and seeks to assess whether it is understandable to someone in that position. Since code is read far more often than it is written, code review can be a very effective tool for maintaining the long-term health of a code-base when applied in this way.