Remove author column from Phabricator table to avoid revision owner vs. commit author confusion
Categories
(bugzilla.mozilla.org :: Phabricator Integration, enhancement, P3)
Tracking
()
People
(Reporter: kats, Assigned: dkl)
Details
(Keywords: conduit-triaged)
Attachments
(1 file)
(deleted),
text/x-github-pull-request
|
Details |
The bugzilla attachment table has an author field for each revision uploaded to phabricator, and this author field shows the name of the user who uploaded the revision. I think it makes more sense for this to show the author field from the commit (i.e. what lando's landing view shows) because sometimes a mozilla staff member is uploading volunteer patches, and the patch appears mis-attributed to the staff member instead of the volunteer.
This is related to https://bugzilla.mozilla.org/show_bug.cgi?id=1538151 in that both bugs are papercuts when it comes to landing volunteer patches, because phabricator treats the "author" of the differential as whoever uploads it, rather than using the author field of the commit.
Comment 1•5 years ago
|
||
Although https://bugzilla.mozilla.org/show_bug.cgi?id=1538151#c8 claims it is a separate bug (or feature, apparently), you practically run into the same issue: Phabricator considers the uploader the revision author, and doesn't allow that person to review, even if he is not the author of the patch.
Updated•5 years ago
|
Phabricator considers the uploader the revision author, and doesn't allow that person to review, even if he is not the author of the patch.
as the revision owner needs to be a phabricator user (because it's a database), assuming the patch author does not have a phabricator account they cannot be set as the revision's owner. if they have a phabricator account they should upload the patch themselves.
getting the author information correct on bmo is tricky - a single diff may have multiple authors, and the author data is unvalidated freeform text.
to avoid confusion we're thinking of removing the author column from the table in bugzilla.
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 3•5 years ago
|
||
Assignee | ||
Comment 4•5 years ago
|
||
Merged to master.
Description
•