Change Details

Discussion about revert handling comes up every so often. Collecting some of that discussion here. Currently, Phabricator treats reverts exactly like any other commit and does no special handling of them. (To be continued!)

(Discussion about revert handling comes up every so often. Collecting some of that discussion here.) Currently, Phabricator treats reverts exactly like any other commit and does no special handling of them. We can probably do better than this. One problem is that the current behavior is counterintuitive -- users often expect to reopen a revision and continue code review there after the changes are reverted, but we don't support a "reopen" operation right now. Reopening is tricky, though, and adds enough complexity that it might not be the right approach to pursue. For example: - `arc diff` won't be able to automatically associate the revert-of-revert commit with the original revision. - The daemons likely need additional complexity to avoid, e.g., closing a reopened revision incorrectly. And, generally, the current model works well enough at a low revert rate (Phabricator's is around 0.3% at the moment). One simpler improvement might be to detect when a commit is a revert (by examining the commit message for, e.g., the git revert template) and note that on the revision, e.g. "reverted by rXnnnn".

(Discussion about revert handling comes up every so often. Collecting some of that discussion here.) Currently, Phabricator treats reverts exactly like any other commit and does no special handling of them. (To be continued!)We can probably do better than this. One problem is that the current behavior is counterintuitive -- users often expect to reopen a revision and continue code review there after the changes are reverted, but we don't support a "reopen" operation right now. Reopening is tricky, though, and adds enough complexity that it might not be the right approach to pursue. For example: - `arc diff` won't be able to automatically associate the revert-of-revert commit with the original revision. - The daemons likely need additional complexity to avoid, e.g., closing a reopened revision incorrectly. And, generally, the current model works well enough at a low revert rate (Phabricator's is around 0.3% at the moment). One simpler improvement might be to detect when a commit is a revert (by examining the commit message for, e.g., the git revert template) and note that on the revision, e.g. "reverted by rXnnnn".