Page MenuHomePhabricator

Hide the "large" diff warning on "very large" diffs
ClosedPublic

Authored by epriestley on Apr 30 2018, 6:55 PM.
Tags
None
Referenced Files
F13035806: D19416.id46443.diff
Sun, Apr 14, 10:41 AM
Unknown Object (File)
Thu, Apr 11, 8:59 PM
Unknown Object (File)
Thu, Apr 11, 8:59 PM
Unknown Object (File)
Thu, Apr 11, 6:51 PM
Unknown Object (File)
Thu, Apr 11, 6:51 PM
Unknown Object (File)
Wed, Mar 27, 7:50 AM
Unknown Object (File)
Wed, Mar 27, 7:50 AM
Unknown Object (File)
Wed, Mar 27, 7:50 AM
Subscribers
None

Details

Summary

Ref T13110. Ref T13130. When a revision is "large" (100 - 1000 files) we hide the actual textual changes by default. When it is "very large" (more than 1000 files) we hide all the changesets by default.

For "very large" diffs, we currently still show the "large" warning, which doesn't really make sense since there aren't any actual changesets.

When a diff is "very large", don't show the "large" warning.

Test Plan
  • Viewed a small diff (<100 files), saw no warnings.
  • Viewed a large diff (100-1000 files), saw just the large warning.
  • Viewed a very large diff (>1000 files).
    • Before: both "large" and "very large" help warnings.
    • After: just "very large" warnings.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

amckinley added inline comments.
src/applications/differential/controller/DifferentialRevisionViewController.php
156–158

And these comments should move to be closer to the code or removed.

163

This became $this->isLargeDiff() in the stacked revision, right?

This revision is now accepted and ready to land.Apr 30 2018, 7:28 PM

I'll just nuke the comments in the followup, I think that ended up cleaner/clearer.

This revision was automatically updated to reflect the committed changes.