Differential D13282

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

Authored by epriestley on Jun 14 2015, 5:11 PM.

Details

Reviewers

btrahan

Maniphest Tasks

T5138: Show unit and lint status messages at each diff

Commits

Restricted Diffusion Commit
rP2a5cf5e1b76c: Separate "Revision" and "Diff" fields in Differential

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

Branch

harbor1

Lint

Lint Passed

Severity	Location	Code	Message
Advice	src/applications/differential/customfield/DifferentialLintField.php:35	XHP16	TODO Comment
Advice	src/applications/differential/customfield/DifferentialUnitField.php:35	XHP16	TODO Comment
Advice	src/applications/harbormaster/event/HarbormasterUIEventListener.php:35	XHP16	TODO Comment

Unit

Tests Passed

Build Status

Buildable 6746
Build 6768: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

epriestley updated this revision to Diff 32109.Jun 14 2015, 5:11 PM

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.

epriestley added a task: T5138: Show unit and lint status messages at each diff.

Herald added a subscriber: epriestley. · View Herald TranscriptJun 14 2015, 5:11 PM

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.

cburroughs added a subscriber: cburroughs.Jun 15 2015, 1:20 PM