Details
Details
- Reviewers
epriestley - Maniphest Tasks
- T13151: Plans: 2018 Week 23 - Week 30 Bonus Content
- Commits
- rP67283c7a4526: Add test plan to differential.revision.search
Loaded a revision in the Conduit UI; observed presence of testPlan field.
Diff Detail
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
Comment Actions
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.