Make it easier to make post-review comments from branch notifications

[Björn Tillenius]

I often review changes to a branch, after they have landed. I get the branch revision notification, and look at the diff. If I find something to comment on, it's not that easy to find the right e-mail addresses to send the comments to. I want to send it to the committer, the reviewer(s), and have a public record of it (which means I sent it to the launchpad-reviews list).

The e-mail address of the commiter is in there, but it's not easy to find. The rest of the addresses I have to look up myself. I'm not sure what the best way of solving this problem is. Maybe include the address of the relevant merge proposal as the Reply-To?

[Tim Penhey (thumper)]

Perhaps a way to handle this would be to have the new revision email sent out
have a reply address of the review team of the branch?

This wouldn't have launchpad track it unless the reviewer of the branch is a
team with a mailing list, but it would help with the "who do I comment to?"

[Jonathan Lange (jml)]

Hmm.

I think it's worth thinking about this for a bit before doing anything. The current revision notification emails could do with some reworking. I'll have a chat to Tim about this.

[Karl Fogel (kfogel)]

FWIW, I think setting the Reply-to header in notification emails to an appropriate review destination is fairly common practice. Each project can decide what the right destination is: if that branch has a review team, then that might be the place, or sometimes there's a single mailing list the entire project uses for reviews. (I'm aware that this is getting into new UI work, so it might make sense to come up with reasonable defaults now, and deal later with the question of providing UI to choose alternatives to those defaults).

Making it be the review team of the branch will encourage people to set up teams with mailing lists for this purpose, which might be a good thing anyway.

[Jonathan Lange (jml)]

Yeah, ok. Let's set the reply-to header to the review team's email address.

[Aaron Bentley (abentley)]

We now provide a link to all relevant merge proposals in the email.

I think ideally, replies would go to the merge proposal address (merge+*@code.launchpad.net).

But note that I said "all relevant": if there's more than one merge proposal, we can't do this. But the user can always click through to the relevant merge proposal.

[Björn Tillenius (bjornt)]

On Fri, Oct 30, 2009 at 04:26:46PM -0000, Aaron Bentley wrote:
> We now provide a link to all relevant merge proposals in the email.
>
> I think ideally, replies would go to the merge proposal address
> (merge+*@code.launchpad.net).
>
> But note that I said "all relevant": if there's more than one merge
> proposal, we can't do this. But the user can always click through to
> the relevant merge proposal.

That sounds like a good first step, and I would consider that enough to
close this bug.

A question, though. Let's say I comment on a merged MP, adding myself as
a reviewer. Will I be able to see all my open post-merge reviews
somewhere?

[Aaron Bentley (abentley)]

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

Björn Tillenius wrote:
> A question, though. Let's say I comment on a merged MP, adding myself as
> a reviewer. Will I be able to see all my open post-merge reviews
> somewhere?

I don't know what you mean by "open", but I suspect the answer is "no".

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

iEYEARECAAYFAkrrJOkACgkQ0F+nu1YWqI13wwCfSXWwvQaDfnHGjn43+Ps1hlYC
dfcAn1XS1Aug3peS/zUumDKxY5/nZAw9
=5Ov/
-----END PGP SIGNATURE-----

[Björn Tillenius (bjornt)]

On Fri, Oct 30, 2009 at 05:39:58PM -0000, Aaron Bentley wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Björn Tillenius wrote:
> > A question, though. Let's say I comment on a merged MP, adding myself as
> > a reviewer. Will I be able to see all my open post-merge reviews
> > somewhere?
>
> I don't know what you mean by "open", but I suspect the answer is "no".

I guess I mean reviews where I am a reviewer and haven't yet voted
accepted, rejected, or abstain.

[Aaron Bentley (abentley)]

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

Björn Tillenius wrote:
> On Fri, Oct 30, 2009 at 05:39:58PM -0000, Aaron Bentley wrote:
>> I don't know what you mean by "open", but I suspect the answer is "no".
>
> I guess I mean reviews where I am a reviewer and haven't yet voted
> accepted, rejected, or abstain.

We wouldn't normally consider it open for review if it's already merged.
   If you want to reopen discussion, you should set the status to "needs
review" or something.

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

iEYEARECAAYFAkrrLHYACgkQ0F+nu1YWqI2qXwCfeIFOFyFCc3oWXAM+603OE3tG
XY0AmgJj1wzD0TyVbrAaxbT7jYBkeoi7
=thrC
-----END PGP SIGNATURE-----

[Adam Candy (asc)]

+1, related to lp:609297, it would be very useful to post-review. The alternative is to setup an additional layer of branches and comment through merge requests, but this is over-the-top in many cases where post-review is enough.