launchpad-dev team mailing list archive

[Martin Pool <mbp@xxxxxxxxxxxxx>]

On 22 October 2010 10:36, Andrew Bennetts <andrew.bennetts@xxxxxxxxxxxxx> wrote:
> Julian Edwards wrote:
> [...]
>> At the very least, if the reviewer did understand the branch, I'd expect a
>> comment to that effect confirming the action taken in the changes.
>
> I agree.  I think it's probably a good idea for reviews to have a “cover
> letter” of sorts too.

Yes, that's crucial.  Jumping straight from the bug that's being fixed
to the diff requires the reviewer to infer a lot of intent and it
means they can only comment on the intent by reading the whole diff.
If the code contributor writes a good clear (detailed but not rambly)
cover letter it makes it more likely they will get good conceptual
review not the disappointing situation Julian described.

Also, many times I've found my own mistakes in the course of writing a
cover letter describing them.

Generally speaking Launchpad reviews do seem to have good cover
letters but perhaps they should be more detailed, or people should
respond more to the letter not the diff?

-- 
Martin