Pushing patch to resolve "approval with nits" causes "Approved" status to be lost
Categories
(Conduit :: moz-phab, defect)
Tracking
(Not tracked)
People
(Reporter: mhentges, Unassigned)
References
(Regression)
Details
I discovered this issue when I saw alwu
's comment in the Phabricator.
There's a common workflow in-tree where a reviewer approves a patch, but still suggests some nitpicks. However, they're still satisfied with the overall patch, and are content with it landing (with or without the nits being resolved).
Unfortunately, with the new 0.1.96 moz-phab
patch, updating an "Approved with nits" patch always overwrites the status with either "Needs Reviewer" or "Changes planned":
- If a reviewer had been provided in the commit message (
r?reviewer
), updating an "Approved" patch causes it to change back to "Needs Review". - If reviewers had been assigned in Phabricator (the commit message doesn't have reviewers), the patch had been "Accepted" (with nits), then updated, it gets changed to "Changes Planned".
I'm not sure what heuristic should be used here to properly detect when to change the Phabricator patch status, but FWIW the recent changes affect some common workflows.
Reporter | ||
Updated•3 years ago
|
Comment 1•3 years ago
|
||
Yeah, this current flow is... not great for people with a non-trivial number of review requests...
Comment 2•3 years ago
|
||
Note that it's not even just "with nits", AIUI this means even rebasing a patch will cause reviewers to be re-flagged.
Comment 3•3 years ago
|
||
This seems like a duplicate of bug 1700485.
Description
•