Page MenuHomePhabricator

Allow assigning a diff to a commit post-facto
Closed, ResolvedPublic

Description

There's a benefit in having commits and diffs cross referenced outside of just autoclosing the diff. Much like the Edit Maniphest Tasks button, can there be one for diffs too?

Revisions and Commits

Event Timeline

nochum created this task.May 4 2016, 2:15 PM
eadler added a subscriber: eadler.May 4 2016, 4:06 PM

While I agree it would be nice to have some control over this in the UI here are two comments:

  1. This can be done via the CLI: ./bin/differential attach-commit
  2. It would help if you could articulate the "benefit in having commits and diffs cross referenced outside of just autoclosing the diff". See https://secure.phabricator.com/book/phabcontrib/article/describing_problems/
nochum added a comment.May 4 2016, 4:10 PM
  1. It would help if you could articulate the "benefit in having commits and diffs cross referenced outside of just autoclosing the diff". See https://secure.phabricator.com/book/phabcontrib/article/describing_problems/

Sorry, I assumed it was obvious/was being lazy.

  1. The cross reference is shown on the commit log in the repository which is very useful when we are looking through history and trying to understand why something was done. Usually we'll find information in diff comments.
  2. The commit is then properly associated with the diff in the task which further assists in analysis of code changes when there are multiple commits/diffs for a single task.
chad added a subscriber: chad.EditedMay 4 2016, 5:50 PM

How are you getting commits into the system that have been reviewed but don't have a revision in the commit message. Manually adding these is functionally a step backwards

nochum added a comment.May 4 2016, 6:18 PM

I don't understand...there's no requirement per se for a commit to have a diff attached so the commits come through with no diff attached. Usually this happens in error because the use doesn't enter the diff id correctly or misspells "Differential Revision" or complete forget to add it in, etc. So the diff is there and was properly reviewed but the association hasn't been made.

chad added a comment.May 4 2016, 6:21 PM

Why are users manually adding this information. It's automatic if you use Arcanist/Differential.

nochum added a comment.May 4 2016, 6:25 PM

arcanist is not a practical tool for some of us (we use hg and patch queues), so we do the annotation manually. Is arcanist considered a must-use for phabricator?

chad added a comment.May 4 2016, 6:29 PM

The concern here is Arcanist already solves this problem, and if you really can't use arcanist, then T5000 solves this problem. Beyond that, this feels like a band-aid and not a proper fix to the issue. There should be automatic, programmatic means of doing this. Having users attach after the fact still relies on them to do this step manually, which is unlikely to be 100% a success.

isfs added a subscriber: isfs.May 5 2016, 12:58 PM
nochum added a comment.Jul 5 2016, 3:32 PM

I'm pretty sure this is fixed now with the Edit Related Objects menu awesomeness! Thanks.

epriestley closed this task as Resolved.Jul 5 2016, 3:52 PM
epriestley claimed this task.
epriestley added a subscriber: epriestley.

Yes, commit/revision and commit/task relationships can now be managed manually from the web UI.