1481543 - detect no-op changes

Status: RESOLVED FIXED

(Conduit :: moz-phab, enhancement, P3)

Description

[:glob ✱]

currently every commit is submitted to phabricator each time, even if it hold no changes or fixup commits

this is either something we'd need to handle ourselves (non-trivial), or a request we could raise with upstream; phabricator itself could handle no-op submissions differently (eg. just log that the same commit was resubmitted, no notifications).

Comment 1

[Jan de Mooij [:jandem]]

This would also be great for when you just want to add a new patch to an existing stack. Right now our options are:

1. Plain moz-phab, to submit everything. This is noisy because it updates and comments on each revision.

2. moz-phab . to submit just the current hg revision. This doesn't set the parent revision so you then have to do that manually on Phabricator.

3. moz-phab .^ . to submit just the last and new revision. This is what I usually do because it sets the parent on the new revision, but it updates the previous one for no good reason.

If we ignored no-op changes we could just use (1) without comment spam.

Comment 3

[Simon Giesecke [:sg] [he/him]]

Byron, thanks for identifying this bug which is indeed quite similar to 1570657 I submitted! However, my intention on 1570657 was a bit different. While the description of this bug is to reduce noise in the Phabricator comments, which I also like to reinforce as being useful, I was caring about identifying the situation locally. While I did not mention this explicitly, I primarily want to know if any changes were done after the last submit to Phabricator, which might even be done with an additional argument --check-only or a separate command check (or a better term than "check").

The larger story is that I ran into a situation where I thought that the version on Phabricator was the same that I had pushed-to-try, but in fact it wasn't. This is notoriously different from typical PR workflows, e.g. on GitHub or GitLab, which I and probably other people are used to, where the review and CI are inseparably linked. This is made even somewhat more confusing by the fact that some CI (static analysis) is actually triggered by Phabricator. Indeed, I would prefer a better integration of both, while I understand that doing a full CI on every submit to Phabricator would not be feasible for mozilla-central at least. This is clearly a separate issue, I am just mentioning this here to make my intention clearer.

Comment 5

[Piotr Zalewa [:zalun]]

Attachment: Bug 1481543 Do not update if commit isn't changed. r=zeid

Refactored tests by removing repetitive commit, revision and diff dicts.

Commit is not submitted when following happens:

• It's SHA1 is the same as in Phabricator Diff
• WIP information is not changed.
• reviewers aren't added to a revision without them
• revision wasn't closed

Comment 6

[:glob ✱]

https://github.com/mozilla-conduit/review/commit/6e2b5642b9bf52cb91e507fd364af452a74db947