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
F16528315: D19416.diff
Mon, Jun 16, 10:40 PM
Unknown Object (File)
Sun, Jun 1, 11:17 PM
Unknown Object (File)
Wed, May 28, 4:51 PM
Unknown Object (File)
Tue, May 27, 12:58 PM
Unknown Object (File)
Tue, May 20, 11:15 PM
Unknown Object (File)
Apr 25 2025, 4:22 PM
Unknown Object (File)
Apr 24 2025, 12:09 PM
Unknown Object (File)
Apr 21 2025, 8:20 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
Branch
large1
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 20255
Build 27492: Run Core Tests
Build 27491: arc lint + arc unit

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.