Page MenuHomePhabricator

Increase how many files can have there diff shown
Needs RevisionPublic

Authored by Paladox on Jun 7 2016, 7:10 PM.

Details

Reviewers
mmodell
Luke081515
Commits
rWDQGb1b825c40771: Refactor editor placeholder translation setup
Patch without arc
git checkout -b D257 && curl -L https://phabricator.wikimedia.org/D257?download=true | git apply
Summary

Increase from 1000 to 2500. Would allow more files to be viewed and
diffed.

Change-Id: Ic7101e8c6300111cdbb52d7b9f3c21fb71c24c57

Diff Detail

Repository
rPHAB Phabricator
Branch
wmf/stable
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 567
Build 856: ci-jessieJenkins
Build 855: arc lint + arc unit

Event Timeline

Paladox retitled this revision from to Increase how many files can have there diff shown.
Paladox updated this object.
Paladox edited the test plan for this revision. (Show Details)

What are the performance implications? Why was it set to 1000 before and how did you come up with 2500? Where does this manifest for the user (where do they see the difference)?

I'm not really sure what the performance implications are. I could lower it to 2000 or 1500 if you like, if you doint like the 2500.

In D257#5247, @mmodell wrote:

Upstream has documentation specifically addressing this issue: https://secure.phabricator.com/book/phabricator/article/differential_large_changes/

that's for differential, is this commit only for diffusion though? /me only quickly looked at path/filename

In D257#5248, @Paladox wrote:

I'm not really sure what the performance implications are. I could lower it to 2000 or 1500 if you like, if you doint like the 2500.

Why? You're just throwing out numbers without any clear reason to pick them.

hashar removed a reviewer: hashar.
In D257#5253, @greg wrote:
In D257#5248, @Paladox wrote:

I'm not really sure what the performance implications are. I could lower it to 2000 or 1500 if you like, if you doint like the 2500.

Why? You're just throwing out numbers without any clear reason to pick them.

Well I updated the composer repo but I included a change that was not done in the upstream and I wanted to make sure it looked ok but since it updated too many files I coulden view the diff.

Tangmo1989 changed the visibility from "Public (No Login Required)" to "Tangmo1989 (Tangmo1989)".
Paladox changed the visibility from "Tangmo1989 (Tangmo1989)" to "Public (No Login Required)".Jun 14 2016, 7:10 AM

How many patchsets do we have, where we hit the current limit?

How many patchsets do we have, where we hit the current limit?

I'm not sure.

greg requested changes to this revision.Jul 5 2016, 6:55 PM
greg edited edge metadata.
In D257#5248, @Paladox wrote:

I'm not really sure what the performance implications are. I could lower it to 2000 or 1500 if you like, if you doint like the 2500.

Going back to this. This is reason enough to not merge this. You don't know if this will crash Phabricator for everyone the first time you open a large diff.

This revision now requires changes to proceed.Jul 5 2016, 6:55 PM
demon removed a reviewer: demon.
greg removed a reviewer: greg.