Page MenuHomePhabricator

Add test plan to differential.revision.search
ClosedPublic

Authored by amckinley on Jul 20 2018, 3:48 AM.
Tags
None
Referenced Files
F15573208: D19518.diff
Mon, May 5, 7:56 PM
F15551438: D19518.id.diff
Sun, Apr 27, 5:00 PM
F15534330: D19518.id46672.diff
Wed, Apr 23, 11:46 PM
F15526697: D19518.id46673.diff
Mon, Apr 21, 10:21 PM
F15526382: D19518.id46672.diff
Mon, Apr 21, 8:42 PM
F15514880: D19518.id.diff
Fri, Apr 18, 6:17 AM
F15511367: D19518.diff
Thu, Apr 17, 1:56 AM
F15474461: D19518.diff
Apr 6 2025, 8:50 AM
Subscribers

Details

Summary

Ref T13151. Ref PHI622.

Test Plan

Loaded a revision in the Conduit UI; observed presence of testPlan field.

Diff Detail

Repository
rP Phabricator
Branch
diff-testplan
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 20474
Build 27810: Run Core Tests
Build 27809: arc lint + arc unit

Event Timeline

I think this is a good practical fix to PHI622, particularly considering that summary is already present.

Two hypothetical future concerns that I think aren't actually worth dealing with today:

  • In theory, this field should enable/disable itself if you enable/disable the "Test Plan" custom field, and the "Test Plan" custom field should provide the value to Conduit without needing any code here. I looked at this briefly but couldn't find a straightforward pathway, I believe. The Differential custom field code has generally been improving through adjacent changes over time so I'm reasonably confident we'll get somewhere sensible eventually.
  • Elsewhere, at least some remarkup-valued fields are futureproofed by returning a dictionary value like {"raw": "zebra"} instead of just a string ("zebra"). One example is the ManiphestTask description field on maniphest.search. This dictionary could support options like "render all remarkup to text/html in the return value", "support storing non-Remarkup values [e.g. commit messages written in Markdown]", and so on. It's not clear to me that this will ever actually be valuable, but we'd be in a bit of trouble today if we decided we do want to accommodate it. But, here, we already have to work with 'summary' so I think we're not in a materially worse position by not adopting this vague possible futureproofing.
This revision is now accepted and ready to land.Jul 20 2018, 3:35 PM
This revision was automatically updated to reflect the committed changes.