Page MenuHomePhabricator

Separate "Revision" and "Diff" fields in Differential
ClosedPublic

Authored by epriestley on Jun 14 2015, 5:11 PM.
Tags
None
Referenced Files
F15485101: D13282.diff
Wed, Apr 9, 9:45 PM
F15478662: D13282.id.diff
Tue, Apr 8, 1:47 AM
F15466288: D13282.id32109.diff
Thu, Apr 3, 3:57 AM
F15459918: D13282.id32199.diff
Mon, Mar 31, 6:14 PM
F15453104: D13282.id32199.diff
Sat, Mar 29, 9:26 AM
F15445935: D13282.id.diff
Thu, Mar 27, 4:38 PM
F15443543: D13282.id32109.diff
Thu, Mar 27, 4:56 AM
F15439077: D13282.diff
Wed, Mar 26, 5:34 AM

Details

Summary

Fixes T5138. Some of the "revision" properties are really "diff" properties, but we only show the properties for the most recent / current diff.

  • Immediately, this makes it hard or impossible to review, e.g., lint/unit results for older diffs.
  • Longer-term, these limits will become more problematic with more data on diffs after Harbormaster.

Instead, separate "revision" from "diff" properties.

(In the long term, it might make sense to show more diffs in this panel -- e.g., tabs for the 8 most recent updates or something -- but I went with the simplest approach for now since I don't have a clean way to deal with 100-update revisions offhand.)

Test Plan

Screen Shot 2015-06-14 at 9.53.57 AM.png (808×1 px, 122 KB)

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

epriestley retitled this revision from to Separate "Revision" and "Diff" fields in Differential.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: btrahan.

In a future change, "Download Raw Diff" could also probably move down, and "Update Diff" might make sense as a button on the lower box. These were slightly nontrivial for now and I'm not totally sure where else the panel is going, so I didn't bother yet.

This revision is now accepted and ready to land.Jun 15 2015, 6:49 PM
This revision was automatically updated to reflect the committed changes.