See PHI483. There's currently a bad interaction between builds completing and drafts unpublishing, described in greater detail in T13108.

T13108 lays this out with richer narrative context. This is going to be something like:

• New Engine for publishing build status changes (or SCOPE EXPLOSION into a new Engine for publishing all changes to all objects?!).
• The Diffusion version of this engine does exactly what it currently does for now (publish a boring transaction to the timeline).
• The Differential version of this engine becomes fancier.
• New bin/harbormaster publish to try to combat the complexity this introduces.
• DifferentialRevision->loadActiveBuilds() becomes more like DifferentialRevision->getKnownFinalBuildStatus() which returns "pass", "fail", or "not yet known". The response logic changes accordingly. This is now decoupled somewhat from build status in Harbormaster when aggregating the results of autoplans.
• Maybe a little later, start sending email to authors on build failures.

See PHI283. When a draft revision is abandoned, it should retain its "draftness" in some sense and not send email. With the above change, revisions should also probably be able to demote into a "Changes Planned + But, Still a Draft" state.

Then:

• Make isSortOfADraftStill a separate flag from the "Draft" status. Maybe PROPERTY_HAS_BROADCAST can represent this exactly, already.
• Condition broadcasting on this flag only, not the actual draft flag.
• When a revision with this flag is abandoned, don't clear the flag.
• Update the UI and API to distinguish between "abandoned" and "abandoned + draft" clearly.
• When a build failure occurs on a non-held draft, demote to "Changes Planned" and retain the flag.

See PHI356, which has some reasonable suggestions for minor improvements to the filetree view.

• When a folder in the filetree contains exactly one child, fold the child into the parent.
• Show which files the viewer has authority over, like the table of contents.
• Maybe some minor CSS tweaks since I'm not entirely happy with where this is at the moment.

See PHI230, where I plan to explore the [----++ ] element described in D16322 and see how it works.

• Put it on the dashboard.
• Maybe put it in email now; maybe wait and see how it goes on dashboards. I'd ideally like to replace "[238 lines]" in email with this element.

See PHI485 and PHI489. An install would like better behavior from Differential when reviewing revisions which change 5,000+ files and have 1,000+ reviewers. See also T8612.

It's no secret that I think these use cases are mostly bad and that these revisions do not actually receive review. However, there are increasingly good reasons to send them through Differential anyway (build integrations, webhooks, submit queue integrations, "regulatory/compliance" reasons, triggering Herald side effects, cohesion-of-process/plain-human-laziness, etc.) fully realizing that no one will really look at the textual changes of the diff.

The strategy I plan to pursue here is to just accept that the changes are not important for these revisions and degrade them to a separate page, while retaining the /Dxyz page for discussion.

These tasks also identify some more-legitimate cases where performance could improve. One example is that if you add 1,000 reviewers to a revision, we render "X added reviewers: ..." and lists all 1,000 reviewers in the timeline. Likewise, the entire set of reviewers is shown on the dashboard. Although adding 1,000 reviewers isn't really "legitimate review", it also isn't absurdly abusive, and our behavior isn't very good and these interfaces should reasonably degrade like Maniphest and Subscribers already do.

• Build a standalone, pagingated page for the table of contents. This just lists changes in a huge table with links to the corresponding standalone views. This can be a SearchEngine page with some filtering options.
• Provide a link from the normal table of contents.
• Above 1K changes, replace the table of contents and changesets with a link to the paginated view.
• Remove the "large changes" warning prompt in arc, which I suspect no human has ever answered "n" to. This will moot T10852.
• Remove the "Differential User Guide: Large Changes" article from the documentation.
• Above 1K changes, probably degrade some other elements like "Other open revisions" and the filetree.
• Summarize large reviewer lists in the timeline and on the dashboard (and, ideally, in the primary list).
• Take any other easy optimization opportunities from the profiles in PHI485.

See PHI431. See PHI426. ... I suspect these lead to T731 and T10574, practically.

These still need a bit more planning but are probably mostly like:

• Add a new ReviewRulesetEngine.
• Revisions default to the normal ruleset ("At least one human reviewer"); Herald global rules can change them to use a different ruleset ("At least 2 human reviewers", "all human reviewers", etc).
• DifferentialTransactionEditor->updateReviewStatus(), which decides whether a review should transition to "Accepted" or not, becomes a callback into the new Engine.
• DifferentialTransactionEditor->expandTransaction(), which includes logic to decide when reviews should be voided, becomes a callback into the new Engine. VoidTransaction probably becomes simpler and the recent logic it gained moves to the Engine instead. This callback should also be able to upgrade reviewers (e.g., accepted -> blocking), force them to resign, etc., not just void them.
• DifferentialReviewer gets more support for primitive behaviors, including timeouts, to allow custom Herald and Review actions to do zanier things.
• The new RulesetEngine gets more UI hooks so it can render "Waiting for 7 more humans and at least 1 representative of the Code Style Review Authority Consulate to accept this revision." and customize how the "Accept" aggregation and "Void/downgrade/upgrade" transitions are rendered.

Compatibility:

• Implementing BuildableInterface now requires implementing newBuildableEngine().
• Implementing BuildableInterface no longer requires implementing getHarbormasterPublishablePHID(). Publishing indirection is now handled by BuildableEngine.
• PhabricatorTransactions::TYPE_BUILDABLE no longer exists. Instead, applications should implement application-specific modular transactions to report build status information. DiffusionCommitBuildableTransaction may be useful as a template.

Compatibility:

• getHasBroadcast(), setHasBroadcast(), and PROPERTY_HAS_BROADCAST are now getShouldBroadcast(), etc.

Compatibility:

• shouldBroadcast() has been removed; getShouldBroadcast() now means the same thing.

Junk:

• Existing drafts need to be migrated to setShouldBroadcast(false) explicitly. This wasn't populated by default before, only on transition.

Some states:

where I plan to explore the [----++ ] element

Here's what this looks like after D19294 on my local test data:

Briefly:

• Revisions are rated from 1-7 stars.
• Scale is logarithmic. You get another star at 20, 50, 150, 375, 1000 and 2500 lines (total lines changed).
• Each star is either a "-" or a "+"; these are linear by how many lines of each type you have relative to one another.
• If you have at most one "+", the element is green.
• If you have 4 or more "+", the element is red.
• All these numbers and all of the behavior are subject to change.
• Mail subject lines now say [Request] [++-- ] instead of [Request, 123 lines].

This script can back-populate revision sizes that haven't been updated since D18832:

<?php

require_once 'scripts/init/init-script.php';

$table = new DifferentialRevision();
$conn = $table->establishConnection('w');

foreach (new LiskMigrationIterator($table) as $revision) {
  $diff = $revision->loadActiveDiff();
  if (!$diff) {
    continue;
  }

  $row = queryfx_one(
    $conn,
    'SELECT SUM(addLines) A, SUM(delLines) D FROM %T
      WHERE diffID = %d',
    id(new DifferentialChangeset())->getTableName(),
    $diff->getID());

  $properties = $revision->getProperties();

  $properties[DifferentialRevision::PROPERTY_LINES_ADDED] = (int)$row['A'];
  $properties[DifferentialRevision::PROPERTY_LINES_REMOVED] = (int)$row['D'];

  queryfx(
    $conn,
    'UPDATE %T SET properties = %s WHERE id = %d',
    id(new DifferentialRevision())->getTableName(),
    phutil_json_encode($properties),
    $revision->getID());
}

It's a little slow-ish and D18832 landed in December 2017 so I'm not necessarily going to make it an upstream migration.

Filetree stuff is now in T13516: Remove the "filetree" setting (so it's always enabled for everyone).