Cannot add another person to a review discussion when reviewing via email

[Björn Tillenius]

There should be an easy way of adding someone to the review discussion via e-mail.

It's quite common that you want to get someone's opinion when doing a review. Normally what you do is that you CC the person when you send a mail to a review-list, and that person gets added to the discussion (assuming that people use reply-to-all, which they normally do)

This doesn't work with merge proposals, though, since if you CC someone, they will get a mail, but the merge-proposal's subscribers wont' see that the person was CC:ed, so they won't include him in the discussion.

It would be nice if simply CC:ing someone would do the right thing, but having a command to subscribe somone would be acceptable.

[Björn Tillenius (bjornt)]

I marked this a an adoption blocker, since I feel that having efficient communication is crucial when doing reviews.

[Matthew Paul Thomas (mpt)]

In bug 295269 Joshua Blount suggested using a Twitter-style @nickname syntax for this purpose. I think that could be risky because code reviews are moderately likely to include @word syntax already (in reviews of CSS code, for example).

[Joshua Blount (jblount)]

I don't think this would be so risky, it seems like it would be a good thing to parse the @foo and look for a user named foo. If this user is found, link the @foo and cc foo on the comment / bug report / review, if the user foo isn't found, then just leave the text as is.

Another way to do this is to use the existing ideas from the launchpad bzr integration stuff ( lp:~foo ). This would be pretty simple for people who are using LP for code hosting / reviews, and would provide the branding benefit of reminding the user of lp while typing it. It also could seem a bit less like we're just integrating "micro-blogging" features into Launchpad and a bit more natural to Launchpad's existing feature set.

[Björn Tillenius (bjornt)]

On Thu, Nov 13, 2008 at 12:51:35PM -0000, Joshua Blount wrote:
> I don't think this would be so risky, it seems like it would be a good
> thing to parse the @foo and look for a user named foo. If this user is
> found, link the @foo and cc foo on the comment / bug report / review, if
> the user foo isn't found, then just leave the text as is.

If we'd do that, I wouldn't want to be this user:

    https://launchpad.net/~property

(Considering that when reviewing Python, writing @property is quite
common).

[Tim Penhey (thumper)]

I think we could subscribe the person CC'ed to the email to the source branch for code review messages.

However I think there should be some caveats:
  * What about a mailing list? Should the team be subscribed?
  * What if the person is already subscribed through a team?
  * What should we do if the email address is unknown? Very hard to return error information.

[Tim Penhey (thumper)]

A command to subscribe someone has less ambiguities, and is equally doable.

[Björn Tillenius (bjornt)]

On Mon, Nov 17, 2008 at 02:35:19AM -0000, Tim Penhey wrote:
> However I think there should be some caveats:
> * What about a mailing list? Should the team be subscribed?

I guess we'd have to subscribe it, since we want the mailing list to be
included in the discussion.

> * What if the person is already subscribed through a team?

Subscribe the person. I (and many other) filter my e-mail differently if
I'm subscribed directly, than if I'm subscribed through a team. The mail
that get sent to me directly (rather than through a team) get much
higher attention. That's the whole point of CC someone, rather than
relying on that he reads the mailing list.

> * What should we do if the email address is unknown? Very hard to
> return error information.

Right, this is a tricky issue. Sadly, I think the only thing we can do
is to ignore the CC, maybe sending something to the one how tried to add
the CC, to tell him that it's not possible.

[Aaron Bentley (abentley)]

We can't cc people without making them reviewers, but we can now make them reviewers via email.