Two things we've intentionally obsoleted and removed, which won't be returning:

• DifferentialFieldSelector mostly no longer does anything, and will vanish completely soon. This will be replaced by standard mechanisms in CustomField. These options aren't available quite yet (they're waiting on the field implementations to stabilize).
• The differential.revision-custom-detail-renderer setting is obsolete. This allowed you to add actions to Differential. Instead, use standard event mechanisms.

Was this the largest 'technical debt' task we have on the books?

Yeah, by far. There will probably be one more ApplicationTransactions migration eventually for Diffusion/Audits, and we have a couple of other things that need general updating (like Owners), but everything else on the books is very small compared to this.

(There are two other tasks -- T1191 and T4045 -- which may involve migration of a large amount of data, but the code changes associated with both of those are trivial.)

We have a bug with "Plan Changes" right now, exemplified in D8366. Generally, there are a bunch of edge cases with state transitions now (T4168, T4167) and some pending work on them (T3202, T2543). All of these weird cases cropped up when we started tracking status individually in T1279.

One possible fix is to separate "needs review" and "plan changes" into different states. Another fix is to invalidate accepts on "plan changes". The latter lets us disable sticky accept more easily.

I think the interesting workflow here is:

• Revision is accepted;
• author plans changes;
• author requests review (without updating, i.e. requests review of the same code).

What state should the revision end up in? If it's "Accepted" (and I think it probably should be), then I think we need a separate "plan changes" state.

Similar to the above, revisions currently aren't kicking back to "Needs Review" after an update if there's at least one "Rejecting" reviewer. This and the above are more-broken-than-intended and will be getting fixed soon, but I'm dealing with a bunch of IRL junk today and may not have a patch until tomorrow.

My specific plan here is:

• Introduce a new "Changes Planned" revision state. This is like "Needs Revision", but requires an explicit transition ("Request Review", "Update") to get out of. This will fix the "Plan Changes" -> "Accepted" transition. It works just like "Needs Revision" in other settings.
• Introduce new explicit reviewer states, "Accepted Older Diff" and "Rejected Older Diff". When a revision is updated with a new diff, all "rejects" will be changed to "rejected older diff". If sticky accept is disabled, accepts will be changed to "Accepted older diff". State-based transitions will then take effect, which may move the revision to "accepted" immediately (if we have a sticky accept and no rejects) or may leave it in "needs review".

Some of Differential's fields are diff-specific, rather than revision-specific: branch, lint status, unit status, host, path, and then sort-of arcanist project and repository (these are transferred to the revision).

Previously, we monkeyed around with these so they showed the information for the visible diff implicitly, instead of information for the active diff. This wasn't really indicated in the UI, but generally aligned with user expectations. However, it was awkward from an implementation standpoint, and it currently no longer happens.

The primary way I want to address this is by making some of these custom fields of Diffs, and putting a second section in where the diff stuff lives, roughly like this:

+-----------------------------------------------+
| D123 do some stuff                            |
+--------------------------------------+--------+
| prop: value                          | action |
| prop: value                          +--------+
| prop: value                                   |
+-----------------------------------------------+
| SUMMARY                                       |
| did some stuff                                |
+-----------------------------------------------+

+-----------------------------------------------+
| Diff 15                                       |
+--------------------------------------+--------+
| lint: value                          | action |
| unit: value                          +--------+
| brch: value                                   |
+-----------------------------------------------+

If that ends up being hideous, I'll merge the diff stuff into the revision stuff instead (like it used to be), but probably separate it out a little bit.

Among these fields, we may also have been special casing lint and unit results so they didn't show results for the automatic updates on close. I don't have a specific plan for this (I'd like to avoid implementing the same magical rule as before, if it did actually exist, since it's not explicit) but will take a look at smoothing things out.

If the revision summary is empty, the mail messages now look a little awkward:

Also, the test plan is now gone from the message -- not sure if that was an intentional removal or an oversight.

Test plan removal was "intentional-until-someone-complains". I take it you'd like to see it return?

I don't feel super strongly, but am happy to see it back -- thanks. :)