#3268 Collapse large diffs by default
Closed: Fixed 5 years ago Opened 5 years ago by amitosh.

It's not very pretty to show a ~9000 lines of diff in a PR. This makes reviewing utterly difficult. Here's one: https://pagure.io/Fedora-app/pull-request/70#request_diff.

Collapsing all diffs with lines ~200 will probably be enough. Any change greater than that has a high probability of being some autogenerated file.


Collapsing all diffs with lines ~200 will probably be enough. Any change greater than that has a high probability of being some autogenerated file.

Having reviewed recent changes that included very large (and valid) diff, I am inclined to not change the current behavior.

@ryanlerch do you want to weigh in here?

Metadata Update from @pingou:
- Issue tagged with: RFE

5 years ago

Having reviewed recent changes that included very large (and valid) diff, I am inclined to not change the current behavior.

I am defining "collapsing" as hiding the diff using JS on the client side, and providing a large prominent expand button, something like GitHub and GitLab.

When we really want to review something that's in the bottom of the list and the huge diff in Pipfile.lock or package-lock.json, forces us to scroll for miles.

IMHO, just having the ability to collapse a diff would be good here, that way the user can see the lines changed i.e. +9000 and make the decision to collapse that diff.

Not sure if adding an arbitary value for auto-collapse is a good idea here.

Login to comment on this ticket.

Metadata
Related Pull Requests
  • #3400 Merged 5 years ago