[TriangleEdge]

Amazon eng did some research and found the number of comments in a code review is proportional to the number of lines changed. Huge CRs get little comments. Small CRs get a lot of comments. At Amazon, it's common to have a 150 to 300 line limit to changes. It depends on the team.

In your case, I'd just reject it and ensure repo merges require your approval.

[kwk1]

"Inversely proportional" for what it's worth

[senderista]

Also, some teams have CR metrics that can be referenced for performance evaluations.

[ekjhgkejhgk]

Could you please provide a reference? I couldn't find it.

[zukzuk]

That’s a great way to discourage anyone ever doing any large scale refactoring, or any other heavy lifting.

[febusravenga]

That's good. Because large refactorings are usually harmful. They are also usually unplanned, not scoped and based on very unquantifiable observations like "I don't like the code is structured" - let's do ity way.

[Cthulhu_]

That's a good thing, large scale refactorings should be very, very rare. Even automated code style changes can be controversial because of the churn they create. For large and/or important software, churn should be left to a minimum, even at the cost of readability or code cleanliness. I've seen enough open source projects that simply state they won't accept refactoring / reformatting PRs.

[gempir]

That means your code will stay old.

A new language feature is released, you cannot apply it to old code, since that would make a big PR. You need to do super slowly over time and most old code will never see it.

A better static type checker, that finds some bugs for you, you cannot fix them as your PR would be too big, you instead would need to make a baseline and split it up endlessly.

In theory yes, maybe a bit safer to do it this way, but discouraging developers to make changes is bad IMO. Obviously depends on your usecase, if you develop software that is critical to people's literal life, then you'll move more carefully.

But I wager 99% of the software the world produces is some commerce software, where the only thing lost is money.

[wizzwizz4]

> A new language feature is released, you cannot apply it to old code, since that would make a big PR.

Good. Don't change code for the sake of shiny new things syndrome.

> A better static type checker, that finds some bugs for you, you cannot fix them as your PR would be too big,

Good. Report each bug separately, with a suggested fix, categorised by region of the code. Just because you ran the program, that doesn't mean you understand the code well enough to actually fix stuff: those bugs may be symptomatic of a deeper issue with the module they're part of. The last thing you need is to turn accidentally-correct code into subtly-wrong code.

If you do understand the code well enough, what's the harm in submitting each bugfix as a separate (independent) commit? It makes it easier for the reviewers to go "yup, yup, yup", rather than having to think "does this part affect that part?".

[ThiefMaster-]

Large-scale refactoring is not something you want from an external contributed, especially not if unsolicited.

Typically such refactoring is done by the core development team / maintainers, who are very familiar with the codebase. Also because DOING such a change is much easier than REVIEWING it if done by someone else.

[TriangleEdge]

The review bots can be bypassed.

[charlieyu1]

You want to do large scale refactoring without the main team agreeing? Seems like a disaster.

[arachnid92]

Just split up your work across multiple PRs.