We have email distribution lists for various code components in our product and requests for review end up being sent to these lists.
Twice this week I’ve sent emails in response to code reviews along the lines of:
“Please re-send your request with enough information to give the potential reviewer a clue what they should be looking at.”
It does not appear obvious to some people to do this. Our distribution lists is serviced by a number of possible people, with one person selected as the “victim of the week” to own answering the email and/or doing whatever defect work ends up generated by that email if any.
For review requests, it may be that somebody who knows the code better than the person monitoring the list for the week can review, but if you don’t describe the change in more detail then that possible “better reviewer” would never know to look.
Leaving half the work to the reader of the email that can be easily justified in a number of circumstances. However, if your email is distributed, then an attempt to save a bit of time in drafting it, means that all the readers incur the costs. The cost of email servicing can really add up, so make them count.
A review request should include the following
- Defect # and abstract if appropriate.
- High level description of what’s being changed, and why.
- Filenames and function names of what is being changed. If this is a lot then details can be provided outside of the email.
- Context diffs (for example those generated by (gnu) ‘diff -p -U10′. Many people have preferences on how they compare files, so also provide the new and original versions of the files. Do not attach these to the email, but include a path to where these are available.
- Instead of providing copies of the files, it may make sense to provide information about the version control sandbox that contains the files. Do this in an effective way ((*) see below).
(*) Perhaps unique to clearcase, there can be bad ways to share information about your changes. It is common for people to share the clearcase branch name that they’ve used. Also provide the viewname (if that view is accessible to your reviewer), and the config spec (assuming your view is not accessible to the reviewer). What your changed files are branched from many not be obvious, and these may not even be visible to the reviewer in their view, if a specific version of the directory element is required for the file to be visible (i.e.: when you have created a new file).
