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
F13769748: D19416.diff
Sun, Sep 8, 12:54 AM
Unknown Object (File)
Fri, Sep 6, 3:18 AM
Unknown Object (File)
Sun, Aug 25, 5:18 AM
Unknown Object (File)
Aug 6 2024, 10:30 PM
Unknown Object (File)
Aug 5 2024, 2:32 AM
Unknown Object (File)
Aug 3 2024, 11:44 AM
Unknown Object (File)
Aug 1 2024, 12:01 PM
Unknown Object (File)
Jul 26 2024, 3:23 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.