1259963 - attachment flags are bound to the wrong element when multiple attachments are created at the same time (eg. from mozreview)

Status: RESOLVED INCOMPLETE

(bugzilla.mozilla.org :: User Interface, defect)

Description

[Mike Hommey [:glandium]]

I don't know where this comes from. If it's mozreview doing something fishy or Bugzilla not handling things properly, but something fishy went on with bug 1259960.

I did a push of 4 changesets to mozreview, standard procedure. Mozreview created 4 attachments, and 4 comments.

What is fishy is that:
- https://bugzilla.mozilla.org/show_bug.cgi?id=1259960 shows the review flags set on the 4 attachments on the message for the first attachment being created, so /before/ the last 3 attachments are mentioned to be created. That's confusing from a bugzilla UI perspective.
- I only received *one* bugmail for the 4 attachments, with all the comments one after the other in the mail (that is not very readable, imho, but that's a separate issue). Where it's weird is that the 4 comments are "Comment #4".

Comment 1

[Mark Côté [:mcote]]

(In reply to Mike Hommey [:glandium] from comment #0)
> I don't know where this comes from. If it's mozreview doing something fishy
> or Bugzilla not handling things properly, but something fishy went on with
> bug 1259960.
> 
> I did a push of 4 changesets to mozreview, standard procedure. Mozreview
> created 4 attachments, and 4 comments.
> 
> What is fishy is that:
> - https://bugzilla.mozilla.org/show_bug.cgi?id=1259960 shows the review
> flags set on the 4 attachments on the message for the first attachment being
> created, so /before/ the last 3 attachments are mentioned to be created.
> That's confusing from a bugzilla UI perspective.
> - I only received *one* bugmail for the 4 attachments, with all the comments
> one after the other in the mail (that is not very readable, imho, but that's
> a separate issue). Where it's weird is that the 4 comments are "Comment #4".

This is the first part of a plan to reduce the noise from MozReview in BMO (bug 1211791).  This particular part was implemented in bug 1237491; the goal of this change was to reduce the number of emails sent out when someone posts a commit set (particularly a large one).  The only way to do this in Bugzilla was to create a special API that sets an identical timestamp for all the activity (attachment and flag creations/updates), which merges the updates into a single bugmail.  I agree it's not terribly readable, but I think it's better than, say, receiving 20 emails every time a 20-commit set is updated.

However as you note there are some side effects that we didn't consider.  I'm starting to wonder if this is too hacky, although I'm not sure what else we can do.  I'll bring it up at the BMO meeting tomorrow.

Comment 2

[Mike Hommey [:glandium]]

> but I think it's better than, say, receiving 20 emails every time a 20-commit set is updated.

I, for one, would actually prefer 20 emails, especially when the commit messages are multi-paragraphs.

Comment 3

[Mark Côté [:mcote]]

What exactly is the issue with multiple paragraphs when the bugmails are concatenated?  Also note that flagmail (e.g. request for reviews) is still sent separately and individually.

We've had several users complain that the volume of bug mail (and comments in general) created by MozReview is too high, to the point that some people don't use MozReview to request reviews.

Comment 4

[Mike Hommey [:glandium]]

(In reply to Mark Côté [:mcote] from comment #3)
> What exactly is the issue with multiple paragraphs when the bugmails are
> concatenated?

That the mail becomes very long.

> We've had several users complain that the volume of bug mail (and comments
> in general) created by MozReview is too high, to the point that some people
> don't use MozReview to request reviews.

That's interesting, since it sends the same amount of messages as attaching patches and reviewing patches in attachments does.

(There is one difference, though: "review granted" messages don't have the comments with mozreview, while they do with e.g. splinter)

Comment 5

[Mark Côté [:mcote]]

(In reply to Mike Hommey [:glandium] from comment #4)
> (In reply to Mark Côté [:mcote] from comment #3)
> > We've had several users complain that the volume of bug mail (and comments
> > in general) created by MozReview is too high, to the point that some people
> > don't use MozReview to request reviews.
> 
> That's interesting, since it sends the same amount of messages as attaching
> patches and reviewing patches in attachments does.

True.  The core problems are (a) MozReview encourages many small commits, which isn't a common practice with BMO patches, and, more importantly, (b) a change to one commit rebases all following commits.  As I understand it there's no fool-proof, or at least straightforward, way to determine if an update is just from a rebase (as evidenced by the various bugs in Review Board's interdiffs).  Thus we decided to try to reduce the noise in and from BMO instead.

Comment 6

[Kohei Yoshino [:kohei]]

MozReview is no longer used, so I think it’s safe to close the bug.