diff --git a/src/applications/differential/conduit/DifferentialCreateRawDiffConduitAPIMethod.php b/src/applications/differential/conduit/DifferentialCreateRawDiffConduitAPIMethod.php --- a/src/applications/differential/conduit/DifferentialCreateRawDiffConduitAPIMethod.php +++ b/src/applications/differential/conduit/DifferentialCreateRawDiffConduitAPIMethod.php @@ -56,10 +56,7 @@ throw new Exception( pht( 'The raw diff you have submitted is too large to parse (it affects '. - 'more than %s paths and hunks). Differential should only be used '. - 'for changes which are small enough to receive detailed human '. - 'review. See "Differential User Guide: Large Changes" in the '. - 'documentation for more information.', + 'more than %s paths and hunks).', new PhutilNumber($raw_limit))); } diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php --- a/src/applications/differential/controller/DifferentialRevisionViewController.php +++ b/src/applications/differential/controller/DifferentialRevisionViewController.php @@ -158,7 +158,7 @@ if (count($changesets) > $limit && !$large) { $count = count($changesets); $warning = new PHUIInfoView(); - $warning->setTitle(pht('Very Large Diff')); + $warning->setTitle(pht('Large Diff')); $warning->setSeverity(PHUIInfoView::SEVERITY_WARNING); $warning->appendChild(hsprintf( '%s %s', diff --git a/src/applications/differential/storage/DifferentialRevision.php b/src/applications/differential/storage/DifferentialRevision.php --- a/src/applications/differential/storage/DifferentialRevision.php +++ b/src/applications/differential/storage/DifferentialRevision.php @@ -742,6 +742,12 @@ return $this->getProperty(self::PROPERTY_LINES_REMOVED); } + public function hasLineCounts() { + // This data was not populated on older revisions, so it may not be + // present on all revisions. + return isset($this->properties[self::PROPERTY_LINES_ADDED]); + } + public function getRevisionScaleGlyphs() { $add = $this->getAddedLineCount(); $rem = $this->getRemovedLineCount(); diff --git a/src/applications/differential/view/DifferentialRevisionListView.php b/src/applications/differential/view/DifferentialRevisionListView.php --- a/src/applications/differential/view/DifferentialRevisionListView.php +++ b/src/applications/differential/view/DifferentialRevisionListView.php @@ -109,7 +109,10 @@ $item->setHeader($revision->getTitle()); $item->setHref($revision->getURI()); - $item->addAttribute($this->renderRevisionSize($revision)); + $size = $this->renderRevisionSize($revision); + if ($size !== null) { + $item->addAttribute($size); + } if ($revision->getHasDraft($viewer)) { $draft = id(new PHUIIconView()) @@ -193,6 +196,10 @@ } private function renderRevisionSize(DifferentialRevision $revision) { + if (!$revision->hasLineCounts()) { + return null; + } + $size = array(); $glyphs = $revision->getRevisionScaleGlyphs(); diff --git a/src/docs/flavor/writing_reviewable_code.diviner b/src/docs/flavor/writing_reviewable_code.diviner --- a/src/docs/flavor/writing_reviewable_code.diviner +++ b/src/docs/flavor/writing_reviewable_code.diviner @@ -74,9 +74,6 @@ We generally follow these practices in Phabricator. The median change size for Phabricator is 35 lines. -See @{article:Differential User Guide: Large Changes} for information about -reviewing big checkins. - = Write Sensible Commit Messages = There are lots of resources for this on the internet. All of them say pretty diff --git a/src/docs/user/userguide/differential.diviner b/src/docs/user/userguide/differential.diviner --- a/src/docs/user/userguide/differential.diviner +++ b/src/docs/user/userguide/differential.diviner @@ -66,8 +66,6 @@ - diving into the details of inline comments in @{article:Differential User Guide: Inline Comments}; or - reading the FAQ at @{article:Differential User Guide: FAQ}; or - - learning about handling large changesets in - @{article:Differential User Guide: Large Changes}; or - learning about test plans in @{article:Differential User Guide: Test Plans}; or - learning more about Herald in @{article:Herald User Guide}. diff --git a/src/docs/user/userguide/differential_large_changes.diviner b/src/docs/user/userguide/differential_large_changes.diviner deleted file mode 100644 --- a/src/docs/user/userguide/differential_large_changes.diviner +++ /dev/null @@ -1,54 +0,0 @@ -@title Differential User Guide: Large Changes -@group userguide - -Dealing with huge changesets, and when **not** to use Differential. - -= Overview = - -When you want code review for a given changeset, Differential is not always the -right tool to use. The rule of thumb is that you should only send changes to -Differential if you expect humans to review the actual differences in the source -code from the web interface. This should cover the vast majority of changes but, -for example, you usually should //not// submit changes like these through -Differential: - - - Committing an entire open source project to a private repo somewhere so - you can fork it or link against it. - - Committing an enormous text datafile, like a list of every English word or a - dump of a database. - - Making a trivial (e.g., find/replace or codemod) edit to 10,000 files. - -You can still try submitting these kinds of changes, but you may encounter -problems getting them to work (database or connection timeouts, for example). -Differential is pretty fast and scalable, but at some point either it or the -browser will break down: you simply can't show nine million files on a webpage. - -More importantly, in all these cases, the text of the changes won't be reviewed -by a human. The metadata associated with the change is what needs review (e.g., -what are you checking in, where are you putting it, and why? Does the change -make sense? In the case of automated transformations, what script did you use?). -To get review for these types of changes, one of these strategies will usually -work better than trying to get the entire change into Differential: - - - Send an email/AIM/IRC to your reviewer(s) like "Hey, I'm going to check in - the source for MySQL 9.3.1 to /full/path/to/whatever. The change is staged - in /home/whatever/path/somewhere if you want to take a look. Can I put your - name on the review?". This is best for straightforward changes. The reviewer - is not going to review MySQL's source itself, instead they are reviewing the - change metadata: which version are you checking in, why are you checking it - in, and where are you putting it? You won't be able to use "arc commit" or - "arc amend" to actually push the change. Just use "svn" or "git" and - manually edit the commit message instead. (It is normally sufficient to add - a "Reviewed By: " field.) - - Create a Differential revision with only the metadata, like the script you - used to make automated changes or a text file explaining what you're doing, - and maybe a sample of some of the changes if they were automated. Include a - link to where the changes are staged so reviewers can look at the actual - changeset if they want to. This is best for more complicated changes, since - Differential can still be used for discussion and provide a permanent record - others can refer to. Once the revision is accepted, amend your local commit - (e.g. by `git commit --amend`) with the real change and push as usual. - -These kinds of changes are generally rare and don't have much in common, which -is why there's no explicit support for them in Differential. If you frequently -run into cases which Differential doesn't handle, let us know what they are.