Closed Bug 1480169 Opened 2 years ago Closed 2 years ago

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


( :: Phabricator Integration, enhancement)

Not set





(Reporter: nika, Assigned: dkl)


(Keywords: conduit-triaged)


(1 file)

45 bytes, text/x-github-pull-request
Details | Review
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.

    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.
Attached file github pr #699
Assignee: nobody → dkl
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
Merged to master.
Closed: 2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.