Consider reducing the verbosity of phabricator 'Revision Approved' bugzilla comments

RESOLVED FIXED

Status

()

RESOLVED FIXED
8 months ago
8 months ago

People

(Reporter: Nika, Assigned: dkl)

Tracking

({conduit-triaged})

Production
conduit-triaged

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

8 months ago
When filing a bug with a large number of revisions on it, the review approved comments can end up taking a large amount of vertical space in the comment.

Currently these comments appear something like the following (from bug 1477432 comment 12):

    Comment on attachment 8993867 [details]
    Bug 1477432 - Part 1: Move xpc_ nsJSID methods to a future-proof API, r=mccr8

    Andrew McCreight [:mccr8] has approved the revision.

    https://phabricator.services.mozilla.com/D2278

    Attachment #8993867 [details] - Flags: review+

The revision link is redundant, as clicking on the attachment would immediately link to the revision anyway. The "has approved the revision" comment is also largely redundant to the automatic flags update note.

It may be useful to keep around the "XXX has approved" line for now, as there is no other place currently where the reviewer's identity is noted, but if bugzilla was to mention the reviewer in the Flags update comment, that line would also be unnecessary, saving a lot of vertical space.
Agreed that the link is not useful; however, we added the "has approved" (in bug 1397927) because the comment comes from Phabricator Automation, not the reviewer, and if there are multiple reviewers it's not obvious which comment belongs to which review.

I admit that I can't remember why we can set the r+ as the correct user but the comment always comes from Phabricator Automation... maybe a limitation in the code that we decided is not worth the effort to fix at the moment.
(Assignee)

Comment 3

8 months ago
Posted file github pr #699
Assignee: nobody → dkl
Status: NEW → ASSIGNED
Keywords: conduit-triaged
Summary: Consider reducing the verbosity of phabricator 'Revision Approved' bugziila comments → Consider reducing the verbosity of phabricator 'Revision Approved' bugzilla comments
Version: Staging → Production
(Assignee)

Comment 4

8 months ago
Merged to master.
Status: ASSIGNED → RESOLVED
Last Resolved: 8 months ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.