Compare two branches
ClosedPublic
Actions

Authored by avivey on Feb 23 2016, 1:38 AM.

Details

Reviewers

epriestley

Group Reviewers

Blessed Reviewers

Maniphest Tasks

T929: Allow two arbitrary revisions to be compared in Diffusion

Commits

rP43f9927a38f7: Compare two branches

Summary

This shows the commits list only (Actual git diff will show up at a later date).
The inputs are left as text-fields, to allow the form to accept anything that can be resolved. The form is GET, to allow sharing URIs.

The conduit method response array is compatible with that of diffusion.historyquery, to make it easy to build
the "history" table.

The hardest part here was, of course, Naming. I think "from" and "onto" are unconfusing, and I'm fairly confident that the "to merge"
instructions are in sync with the actual content of the page.

Test Plan

Look at several "compare" views, with various values of "from" and "onto".

Diff Detail

Repository

rP Phabricator

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

avivey updated this revision to Diff 36973.Feb 23 2016, 1:38 AM

avivey retitled this revision from to Compare two branches.

avivey updated this object.

avivey edited the test plan for this revision. (Show Details)

avivey added a reviewer: epriestley.

avivey added a task: T929: Allow two arbitrary revisions to be compared in Diffusion.

Herald added a reviewer: Blessed Reviewers. · View Herald TranscriptFeb 23 2016, 1:38 AM

Herald added a subscriber: Korvin. · View Herald Transcript

src/applications/diffusion/controller/DiffusionCompareController.php
235	I've copied all of this code from Browse view, I think, with the TODO.

Another big TODO is "support hg, svn".

chad added a subscriber: chad.Feb 23 2016, 3:40 AM

Can diffusion.revlist just be diffusion.historyquery with an extra against argument or similar?
What is cherryPicked?
Can we get all the data in a single command by using --left-right and --format=%m:%H:%P or similar?
Do users really not know how to merge?

Also, your custom art is beautiful.

In D15330#176399, @epriestley wrote:

Can diffusion.revlist just be diffusion.historyquery with an extra against argument or similar?

It's mostly identical, so I guess so.

What is cherryPicked?

The idea is to use --cherry-mark to show that some commits have already been cherry-picked on their own to the destination. But I didn't do the UI and removed the implementation (--cherry-mark was introduced in 1.7.5).
I still think it's a useful signal in this context, and would like to someday (Maybe soon) display it.

Can we get all the data in a single command by using --left-right and --format=%m:%H:%P or similar?

I take a bunch of data for this:

list of commits (rev-list --right-only) with limit
count of commits (rev-list --left-right --count) - we can join these 2 to one rev-list call with no limit (And limit later, or blast the UI).
merge-base - rev-list can't expose this, but in theory if we calculate this one first, all rev-list stuff would be faster / can be resolved from it? Or maybe I don't really want it.

In the most "pure" sense, only rev-list --right-only is "really" required, but I think the use-cases I have in mind ("Show me the difference before I merge to production") would generally benefit from the extra 3 value (Size of this merge, size or reverse merge, Last thing we merged). I think.

Do users really not know how to merge?

That was an attempt to explain what 'from' and 'onto' means... not sure if it made it any better or worse? More text? Pictures? I'd like a picture, but my skills are what produced that lovely custom art...

To be fair, I'm pretty sure that locally I'll render the same information a little differently (Showing related JIRA tickets, names of all reviewers, maybe even pull a custom "Release notes" field), and just use the same conduit method; This view is more of a first-pass than a full-feature.

So, it might makes sense to:

Remove the "reverse merge" completely, from the UI and method, as well as the commit count,
Move this back into diffusion.historyquery, with against argument,
if against argument is provided, also return merge-base value (At the cost of another git call).