498750 - Direct replies to (reviewer) comments should be sent to the comment's author

Status: NEW

(Bugzilla :: Creating/Changing Bugs, enhancement)

Description

[Jens Hatlak (:InvisibleSmiley)]

If I request review for a patch the reviewer is informed via email. But if the reviewer answers on the bug and I reply to that comment the reviewer isn't notified.

Of course I could CC the reviewer (the reviewer probably doesn't want that!) or ask on IRC or send a PM but none of that should be needed. In fact this case is so common it shouldn't require any action from my side.

Suggestion: Parse the comment number from the first line of a comment if it matches "(In reply to comment #n)" where n is a number. Then determine the author of the matching comment and add him to the list of people the new comment should be sent to (unless he's already in the list of course).

The above would actually work for any kind of commenter. If desired the behavior could be limited in such a way that only reviewer comments would be taken into account: Parse the attachment number from the first line of the matching comment (found using the logic above) if it matches "(From update of attachment m [details])" where m is a number. Then check the review flags of that attachment. Only add the author of the comment to the list of people the new comment should be sent to if one of the pending review requests is for him.

Comment 1

[Max Kanat-Alexander]

Well, simpler would be to automatically CC any commenter not CC'ed, when replying to their comment, using JS. It's not that reliable to try to parse out whether we're replying or not, on the server side.

Comment 2

[Gervase Markham [:gerv]]

Or, have a new parameter, temp_cc, to which the commenter is added (using JS, as Max says). So they get bugmail for the immediate reply, but not for future comments unless they are also direct replies to something the person has said. The bugmail could include a "Note: you are not CCed on this bug" notice. This optimises for the short-term rather than the long-term interaction case; I can see people getting quite irritated by being auto-CCed to a number of bugs.

Gerv

Comment 3

[Frédéric Buclin]

No, I don't want a parameter for that. We should stop adding parameters for each minor thing in Bugzilla.

Comment 4

[Gervase Markham [:gerv]]

(In reply to comment #3)
> No, I don't want a parameter for that. We should stop adding parameters for
> each minor thing in Bugzilla.

You know I mean URL parameter and not Bugzilla Administration parameter, right?

As in process_bug.cgi?status=NEW&cc=[e-mail]&temp_cc=[e-mail]&...

with temp_cc being a hidden field, filled in by JS whenever you click a "reply" link.

Gerv

Comment 5

[Gervase Markham [:gerv]]

LpSolit: ping re: comment #4. I do think this is a good idea. I'm sure I've accidentally dropped at least one bug when I wasn't CCed and I thought I was, and people have replied to my comments and I've seemed to ignore them.

Gerv

Comment 6

[Max Kanat-Alexander]

Hmm. I think a better and more Bugzilla-y solution might be to put the "Add me to CC" checkbox also on attachment.cgi, so that people's "add me to bugs I change" preference would keep operating.

Comment 7

[Gervase Markham [:gerv]]

I think that only solves part of the problem. In general, it seems very wise to me that the author of a comment should always receive email which replies directly to that comment - because that is information which is almost certainly of interest to or directed straight at him. And they should do so without being CCed on the bug permanently if they are not anyway. Often a person is involved in a bug for just a short time (I get this a lot with license questions).

So I like the design of using JS on the show_bug page to add a new temp_cc parameter which CCs the person on that change only. We can use the same internal mechanism to temp_cc the creator of a flag when that flag gets set or unset, but that would happen without UI.

Gerv

Comment 8

[Max Kanat-Alexander]

Well, I think a better solution is to start storing a reply_to column in the database, perhaps. It may have to be a table, if we decide that one comment can be a reply to multiple comments, but we can start simple.

Comment 9

[Gervase Markham [:gerv]]

Max: how would a reply_to column be used to solve this problem? Are you saying that instead of filling in temp_cc using JS, we fill in a new reply_to param using JS, and use that to query the database for the comment author, and temporarily CC him that way? If so, that sounds fine to me.

Gerv

Comment 10

[Max Kanat-Alexander]

(In reply to comment #9)
> Max: how would a reply_to column be used to solve this problem? Are you saying
> that instead of filling in temp_cc using JS, we fill in a new reply_to param
> using JS, and use that to query the database for the comment author, and
> temporarily CC him that way? If so, that sounds fine to me.

  Yeah, exactly. He would be put in $forced when passed to BugMail.pm.