upvote

points

by grey-area3 hours ago |
comments
upvoteby photios3 hours ago|
next
[-]
Fully agree with a twist - ban dumb large PRs.

To add to my openspec shilling above, I've been known to "fix" large PRs by reverse engineering specs from them and building each spec out in a separate PR.

Last week me and a coworker turned a 15k line PR into 10 x 1.5k line ones that went in smoothly.

reply
upvoteby grey-area2 hours ago|
parent|
[-]
Yes I do like to make a big pr first sometimes then it's usually pretty easy to split it into small ones and it's a helpful process in many ways. You can even present both together so people can see it either way, but I think in most systems it's better to have smaller prs merged one at a time.

For a rewrite, I guess sometimes it's better to have a spec then write to that, and PRs might be larger.

reply
upvoteby tracerbulletx3 hours ago|
prev|
[-]
So what do you think of Discord rewriting in rust, and AWS shipping an emulator that was ai generated? Do you think those were done with small prs? Do you think that it's bad they did those things?
reply
upvoteby forbiddenvoid3 hours ago|
parent|
next
[-]
Please add your own thoughts to the conversation rather than aggressively asking rhetorical questions. "Counterexamples exist" is hardly a valid criticism of another human's perspective.
reply
upvoteby tracerbulletx3 hours ago|
parent|
[-]
If someone says PRs need to be small, and I say here are some examples of huge PRs that I think are good, do you think these are still bad? That is not remotely invalid.
reply
upvoteby Jtsummers3 hours ago|
parent|
next
[-]
Where did grey-area say they need to be small. As in an absolute? They didn't, you just assumed that's what they meant and then went off the rails in your response.
reply
upvoteby tracerbulletx3 hours ago|
parent|
[-]
The first 4 words? "Just ban large prs." A ban is pretty absolute language. He would have banned my example PRs clearly. Or maybe he wouldn't which is why I asked about them as examples. Because that is the obvious follow up. If you ban large PRs what are you losing? You're loosing big swing aggressive efforts to make sweeping progress.
reply
upvoteby Jtsummers3 hours ago|
parent|
[-]
> The first 4 words? "Just ban large prs."

That was a comment written in a particular context, where the original commenter was concerned with large PRs.

>> how can we review gigantic PR generated by AI.

It was not a comment written in a vacuum saying that all PRs should be small (or not large, whatever that means). Though grey-area does provide some reasonable commentary for why it's good to shrink them (easier to roll back, easier to isolate bugs, etc.). Maybe grey-area does mean to ban all large PRs, but that's not evident in this thread where they wrote a response to a specific comment offering their take on how to resolve the problem raised in that first comment.

> You're loosing big swing aggressive efforts to make sweeping progress.

Just a tip: "losing" not "loosing". We loose arrows at our enemies, we let loose the dogs of war, we loosen our purse strings. But we do not "loose big swing aggressive efforts", we "lose big swing aggressive efforts". It's funny that you get it right once, but mix the two up in the very next sentence.

reply
upvoteby nvme0n1p13 hours ago|
parent|
prev|
[-]
It's like responding to "you should wear a seatbelt, statistically they keep you safer in a crash" with "here are two people who got in a crash without a seatbelt and survived"
reply
upvoteby yjftsjthsd-h2 hours ago|
parent|
prev|
next
[-]
I, personally, would certainly argue that those should be done in small PRs. If you have a working application and want to rewrite in rust, I would expect it to be done by replacing individual components one at a time while making sure relevant tests keep passing. If you're making a new application, I would expect it to be built incrementally by adding parts incrementally.
reply
upvoteby benjiwdubs6 minutes ago|
parent|
[-]
Yeah, if you ended up doing it as a single massive PR how can you ever be sure that existing functionality was preserved?
reply
upvoteby grey-area3 hours ago|
parent|
prev|
[-]
I think they could do those things with small PRs if they chose. If their agents are not intelligent enough to make small prs, I guess they can YOLO and assume everything is just fine with what the agents did. I do think small PRs would be better for agent review as well though, because they avoid any problems with context being overwhelmed and things being missed/forgotten.

What are the PRs you're thinking of though. If you mean a full clean rewrite, they'd probably proceed with spec first then just write it in bits to the spec. No reason it has to be large prs or smaller that's a design choice.

Often an exercise I find useful is do a quick big PR, then split it into small chunks that make sense independently for review. Or you can chain prs. Both those things can be done.

You're welcome to do large PRs, I'm not a fan of huge PRs though and would not pretend to review them (as pretending is what it would be), nor would I like to work somewhere where this is the culture.

reply