This is a huge pet peeve of mine. At work I'm an expert on part of the code base that sees a fair number of contributions.I get many private IMs from colleagues asking me to "Approve please" or something like that with a dump of 100s of lines, of which maybe 10 lines are relevant to me (touch files or behaviour that I'm an expert on, hence why they need my approval.)
Minimally, I would like context for the change, why it required a change to this part of the codebase, and the thought process behind the change. Sometimes but not often enough I send the review back and ask them for this info.
IMO many software developers are just not fast enough at writing or language so providing an explanation for their changes is a lot of work for them. Or they are checked out and they just followed the AI or IDE until things worked, so they don't have explanations to provide. People not taking the time is what makes reviews performative.
> Minimally, I would like context for the change, …
… the why is important
> IMO many software developers … don't have explanations to provide. People not taking the time is what makes reviews performative.
… a lot of developers only consider the how.
i’ve had a lot of experiences of “once my PR is submitted that’s my work/ticket finished” kind of attitude.
i spent a year mentoring some people in a gaming community to become dev team members. one of the first things i said about PRs was — a new PR is just your first draft, there is still more work to do.
it helped these folks were brand spanking new to development and weren’t sullied by some lazy teacher somewhere.
> … the why is important > … a lot of developers only consider the how.
The why is someone else's job, so the developers should just ask them for a blurb to put in the PR for context, along with a note to the reviewer to ask that person for even more context if necessary.
I think there's a why with regard to the code. Why this "how" and not some other "how". (Why did you pick this algorithm, this pattern, this solution to the bigger business why.)
My team uses a github PR template with the following sections. Answers to each can be short yet it has been extraordinarily helpful to pass over important info to the reviewer that's not captured in code. It also borders on "checklist" that the code author has actually done the bare minimum to think things through.
# Goal (why is this change needed at all)
# What I changed and why I did it this way
# What I'm not doing here and how I'll follow up
# How I know it works (optional section, I include this only for teams with lots of very junior engs: "added a test" is often sufficient)
That's a great idea, I should do that. There is another team that does something similar but everyone complains about it (it's not as good as your template).
> IMO many software developers are just not fast enough at writing or language
I think this is the overwhelming factor, software engineering doesn't select for communication skills (and plenty of SEs will mock that as a field of study), or at least most SEs don't start out with them.
> SEs will mock that as a field of study
Who are these people? I've never encountered that. In my experience engineers who aren't great at communication freely own up to it.
Commit descriptions are criminally under used for this purpose. You can add so much more context if you don't limit yourself to just the short commit message.