Page MenuHomePhabricator
Files F15448177

D19298.diff
No OneTemporary
Actions

D19298.diff
View Options

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 <strong>%s</strong>',
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: <username>" 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.

File Metadata

Mime Type
text/plain
Expires
Sat, Mar 29, 3:21 AM (1 w, 6 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7712727
Default Alt Text
D19298.diff (7 KB)

Event Timeline

Phabricator · Built by Phacility