Page MenuHomePhabricator

Help arcanist keep track of which branch matches which review on Git
Closed, ResolvedPublic

Description

When using Git, if I rebase my work on the current master, I need to specify arc diff --update DXXXX for it to understand that I want to update an existing review and not open a new one.

Arcanist could store in git's local config the id of the review and use that, say, run git config --local --add branch.<current_branch>.review DXXXX and check git config --local branch.<current_branch>.review to see if there is a review on that branch already, and use this when I run arc diff.

Event Timeline

mat813 raised the priority of this task from to Needs Triage.
mat813 updated the task description. (Show Details)
mat813 added a project: Arcanist.
mat813 added a subscriber: mat813.

Use arc which to understand what arc diff will do and why.

arc already uses commit hashes (works better for immutable workflows) and "Differential Revision: ..." in messages (works better in mutable workflows) to track revisions. We do not use branch names because some users put multiple changes in the same branch.

Use arc which to understand what arc diff will do and why.

arc already uses commit hashes (works better for immutable workflows) and "Differential Revision: ..." in messages (works better in mutable workflows) to track revisions. We do not use branch names because some users put multiple changes in the same branch.

Mmmm, ok, so, let me say again what I do:

  • git checkout -b some_awesome_feature
  • [code, code, commit, code, commit]
  • arc diff # creates a new review
  • # someone else works on the same files as I do
  • git rebase master #fixup conflicts
  • [more code, commit]
  • arc diff # creates a new review -> bad

arc uses commit hashes, which is great, until I run git rebase because they get thrown away and new hashes are generated. Thus, what I was waying on the first line of my "if I rebase" :-)

Mmmm, do you mean if I put "Differential Revision: DXXX" as the last line of the first commit of the branch, it'll be seen by arc and any subsequent arc diff will update the right review instead of creating a new one ?

Mmmm, do you mean if I put "Differential Revision: DXXX" as the last line of the first commit of the branch, it'll be seen by arc and any subsequent arc diff will update the right review instead of creating a new one ?

If you put "Differential Revision: Dxxx" on any commit in the range, it will update the right review. arc diff should do this for you automatically by default.

mat813 claimed this task.

Mmmm, we have history.immutable: true in our .arcconfig, so that's why arc diff is not doing it, I'll investigate a bit more, thanks :-)

The assumption is that you won't mix history.immutable (which compels arc not to mutate history) with rebasing (which mutates history): if you mutate history, we assume you're comfortable letting arc mutate history too. If you configure it as you have, you're locking us out of both commit messages (which we're forbidden from mutating) and commit/tree hashes (which may change after rebasing), so we don't have much to go on.

That's a bit why I was saying some kind of per branch git config thing could be used too.

I think we have history.immutable because we don't want arc to rewrite the whole commit with the "arcanist commit format", though I think it would be allright if it stopped at adding a Differential Revision: DXXX line at the end.

Yeah, I just did some testing, I think we have history.immutable because otherwise arcanist will completly rewrite the first commit with all it's stuff that we don't want :-)