Launchpad itself

it would be nice if code review email threaded from the initial email about the merge request

Bug #240564 reported by James Henstridge
4
Affects Status Importance Assigned to Milestone
Launchpad itself
Fix Released
High
Aaron Bentley
Launchpad itself 2.1.9

Bug Description

The code review emails from edge correctly generate In-Reply-To headers so that the messages are threaded by mail readers. However, there doesn't appear to be any linkage between the first review comment and the notification of the new merge proposal.

I received the following two emails that should have been threaded (only showing relevant headers):

    X-Launchpad-Branch: ~storm/storm/trunk
    X-Launchpad-Message-Rationale: Subscriber
    To: James Henstridge <email address hidden>
    From: Thomas Herve <email address hidden>
    Subject: Merge of ~therve/storm/zstorm-strong-ref into Storm Development
        Branch proposed
    Message-Id: <20080616124230.21283.94770.launchpad@<email address hidden>>
    Date: Mon, 16 Jun 2008 12:42:30 -0000
    Reply-To: Thomas Herve <email address hidden>

And:

    Reply-To: <email address hidden>
    Message-Id: <email address hidden>
    X-Launchpad-Message-Rationale: Subscriber
    X-Launchpad-Branch: ~storm/storm/trunk
    To: James Henstridge <email address hidden>
    From: Christopher Armstrong <email address hidden>
    Subject: Is this subject required?
    Date: Mon, 16 Jun 2008 12:58:57 -0000

When reply-by-email is enabled for code reviews, it'd also be very nice if the first email had a Reply-To that went to Launchpad rather than the person creating the proposal.

Using similar subject lines for the emails would be useful too, for mail clients that use them as an input for their threading algorithm (e.g. gmail).

Tags: email lp-code
Revision history for this message
Tim Penhey (thumper) wrote :

This would require a db patch to store the message id of the initial email that we send out.

It is quite trivial to do this after we have that message id stored.

Changed in launchpad-bazaar:
importance: Undecided → Low
status: New → Confirmed
Revision history for this message
Björn Tillenius (bjornt) wrote :

Could we raise the priority of this bug please? I'd say this is a blocker for using LP for code reviews for us.

BTW, James, I can't see any In-Reply-To headers in the notifications. You sure you didn't confuse it with Reply-To?

Revision history for this message
James Henstridge (jamesh) wrote :

Bjorn: there are definitely In-Reply-To headers on some of the code review emails. From one of the ones in my inbox:

    X-Launchpad-Message-Rationale: Subscriber
    X-Launchpad-Branch: ~ubunet-pqm-team/ubunet/trunk
    In-Reply-To: <email address hidden>
    Reply-To: <email address hidden>
    Message-Id: <email address hidden>
    X-Launchpad-Project: ubunet

You won't see an In-Reply-To header on the first message though, which is the threading issue I mentioned.

The Reply-To issue was that the "merge proposal update" emails were not set to reply to the mp+NNN email, so you couldn't have a review recorded by simply replying. That issue appears to have been fixed now though.

Revision history for this message
Tim Penhey (thumper) wrote : Re: [Bug 240564] Re: it would be nice if code review email threaded from the initial email about the merge request

On Friday 15 August 2008 13:11:25 James Henstridge wrote:
> Bjorn: there are definitely In-Reply-To headers on some of the code
> review emails. From one of the ones in my inbox:
>
> X-Launchpad-Message-Rationale: Subscriber
> X-Launchpad-Branch: ~ubunet-pqm-team/ubunet/trunk
> In-Reply-To:
> <email address hidden> Reply-To:
> <email address hidden>
> Message-Id:
> <email address hidden>
> X-Launchpad-Project: ubunet
>
> You won't see an In-Reply-To header on the first message though, which
> is the threading issue I mentioned.
>
> The Reply-To issue was that the "merge proposal update" emails were not
> set to reply to the mp+NNN email, so you couldn't have a review recorded
> by simply replying. That issue appears to have been fixed now though.

Aaron has been working on this. The DB patch is in the PQM queue, and
the code has been reviewed and should land tomorrow.

Revision history for this message
Tim Penhey (thumper) wrote :

This should be working, but unfortunately it isn't right now.

Changed in launchpad-bazaar:
assignee: nobody → abentley
importance: Low → High
Aaron Bentley (abentley)
Changed in launchpad-bazaar:
milestone: none → 2.1.9
status: Confirmed → Fix Committed
Aaron Bentley (abentley)
Changed in launchpad-bazaar:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.