Comment 12 for bug 492145

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Martin Pool wrote:
> 2009/12/8 Aaron Bentley <email address hidden>:

>> Separately, ~bzr-core is subscribed to code review email on lp:bzr,
>> which means that its members receive email about code review involving
>> lp:bzr, whether or not they have been requested to review.
>
> So this means they get mail starting "$person has proposed merging ..."?

Yes.

> I'm asking that the mail should make clear:
>
> * who has been asked to review it: you personally, a team to which
> you belong, or something else

I believe this request is based on the misapprehension that it is
possible for this kind of mail to be sent when the receipient has not
personally been asked to do the review.

I think that clarification like "You, not a team you are in, have been
requested to review..." would actually encourage people to believe that
a similar message *could* be sent when a team that they were in was
requested to perform a review.

We could tweak the wording slightly, but if people accept the plain
meaning of the phrase "You have been requested...", they will correctly
understand the situation. I don't see a great need for clarification.

> * whether this is a totally new mp, or a request for a review on a
> new mp, a request for additional review of something already reviewed
> by other people, a request for a re-review of something I myself
> looked at before, a new diff of an existing mp, or a resubmission
> replacing an existing mp

We can certainly look at providing more context here.

>>> If there was a separate cover letter field, we would need to work out
>>> how that meshes with the intended commit message.
>> I don't think they're similar. They have different formats and target
>> audiences.
>
> ... but they're covering similar information?

There is a bit of overlap in content. The commit message usually covers
only what was changed. The cover lettermay cover "what" at greater
length, why the change was made, how it was made and where it was made.

>>> Perhaps it should
>>> be cast as being supplementary to the commit message and also empty.
>> I think that the cover letter is essential, while the commit message is
>> not. Why do you think it should be supplementary?
>
> Eventually, if the mp is accepted, it will have a commit message - at
> least it will be put into the commit of the merge, even if Launchpad
> never knows about it. So at least at that conceptual level, the
> commit message is the key thing.

I don't think Code Review should consider it key if Launchpad need never
know about it. It may make sense to review the commit message at the
same time as the code, to ensure that it, too is up to quality
standards, but it's not an essential ingredient.

For code reviews of any complexity, the cover letter is at least a
helpful guide, and often an absolute necessity. I think that making
them supplementary would make code review much harder to do.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkse0q4ACgkQ0F+nu1YWqI2TzwCdHP5xA4pbkzQWHRfFWugofFYj
NLQAn1f/aZnCcvb/CpJPVJO1z7fbm3zE
=QhNA
-----END PGP SIGNATURE-----