Ideally, you could find pieces of that 10k that would also work as a standalone improvement to the app. I understand that team cultures can vary, but doing small features or refactors in service of a larger goal is nice, because while the reviewer knows you must be up to something, they can still approve your preparatory PRs as face value beneficial.
I have some disagreement here. There are factors other than just code review to worry about when implementing changes, quite notably quality assurance.
It does not one any favours if your 10k LoC gets split in 5 changes that aren’t supposed to regress anything (but need to be validated not to) then 1 tiny on that brings things together.
Some features will be confusing for end users if you drip feed them. We had a whole host of changes recently overhauling our moderation system to be able to track and audit compliance with DSA and the key factor is ensuring the system makes sense to our users and that they can enable, have documentation and on-boarding materials for the changes in functionality and that it’s all QA tested.
In this case we did still review smaller chunks of code, we accumulated them into 1 large merge request at the end and merged it after QA.
That’s a good point; it depends on the extent to which you can make either invisible changes or you can roll out improvements that don’t require coordinated communication.
It's easier to justify in a fast-moving greenfield code base with a verbose language... but I won't defend it. I've gotten better and I'm still getting better at breaking these things up.
I brought the 10-12kLOC PR up as an example supporting my point of view. I don't encourage the behaviour. Most of my PRs these days fall under the 1500LoC mark, tops -- maybe a bit more if it's a tricky component that needs a ton of tests.
10k in 2hrs is 1.5 lines of code per second for 2 hours straight without spending any time to make comments, think about what the code is doing, etc.
In pre-ai era that is just skimming and trusting the person who wrote it or the code changes are largely auto-generated or there exists an exceedingly simple test suite that is incredibly verbose.
Post-ai you are ruining your code base, I probably have to spend 3-5x longer reviewing ai generated code, the code they write tends to be too verbose, mediocre, filled with subtle bugs, adds unnecessary comments, etc. If someone gives me 10k loc pr it's a sure thing they've just let the ai run loose and I'd just tell them what they need to change in general terms instead of wasting days of my time reviewing junk.
It makes no sense. If a feature takes 10k loc to implement, it'd be a huge disservice for the reviewer and yourself to split it up into multiple PRs.
Your comment is equivalent to saying that no feature ever takes 10 loc to implement. It'd be quite ridiculous to say that aloud.
Ideally, you could find pieces of that 10k that would also work as a standalone improvement to the app. I understand that team cultures can vary, but doing small features or refactors in service of a larger goal is nice, because while the reviewer knows you must be up to something, they can still approve your preparatory PRs as face value beneficial.
I have some disagreement here. There are factors other than just code review to worry about when implementing changes, quite notably quality assurance.
It does not one any favours if your 10k LoC gets split in 5 changes that aren’t supposed to regress anything (but need to be validated not to) then 1 tiny on that brings things together.
Some features will be confusing for end users if you drip feed them. We had a whole host of changes recently overhauling our moderation system to be able to track and audit compliance with DSA and the key factor is ensuring the system makes sense to our users and that they can enable, have documentation and on-boarding materials for the changes in functionality and that it’s all QA tested.
In this case we did still review smaller chunks of code, we accumulated them into 1 large merge request at the end and merged it after QA.
That’s a good point; it depends on the extent to which you can make either invisible changes or you can roll out improvements that don’t require coordinated communication.
QA should be happening either way.
It's easier to justify in a fast-moving greenfield code base with a verbose language... but I won't defend it. I've gotten better and I'm still getting better at breaking these things up.
I brought the 10-12kLOC PR up as an example supporting my point of view. I don't encourage the behaviour. Most of my PRs these days fall under the 1500LoC mark, tops -- maybe a bit more if it's a tricky component that needs a ton of tests.
you can read a 10k pr in ~1-2hr. there’s nothing wrong with a 10k pr. i would rather review 1 10k pr than 10 1k prs or 50 200 line prs.
attitudes like this make it seem like computers are incomprehensible and we’re lucky to ever land code at all
10k in 2hrs is 1.5 lines of code per second for 2 hours straight without spending any time to make comments, think about what the code is doing, etc.
In pre-ai era that is just skimming and trusting the person who wrote it or the code changes are largely auto-generated or there exists an exceedingly simple test suite that is incredibly verbose.
Post-ai you are ruining your code base, I probably have to spend 3-5x longer reviewing ai generated code, the code they write tends to be too verbose, mediocre, filled with subtle bugs, adds unnecessary comments, etc. If someone gives me 10k loc pr it's a sure thing they've just let the ai run loose and I'd just tell them what they need to change in general terms instead of wasting days of my time reviewing junk.
> Post-ai you are ruining your code base
if a pr is bad, i will reject it. that's the point of code review.