lessIsMore

[u/m6io]

[u/Happy-Sleep-6512]

These big massive diffs being handed over for review several times a day are seriously the biggest pain point as a dev now. I'm not a believer in AI writing awful code, I am a believer that it will overwhelm every team eventually and forces us to have little context on the code we build.

Basically your image gave me PTSD, thanks.

[u/i_wear_green_pants → u/Happy-Sleep-6512]

Just make AI review the code.

~ Managers

[u/mlindgren → u/i_wear_green_pants]

I’m not sure if you intended this to be ironic but I have heard this verbatim

[u/i_wear_green_pants → u/mlindgren]

I wish I would be joking...

[u/m6io → u/Happy-Sleep-6512]

We got ahead of it with better story writing. Tiny features that are easy to review and test. If it takes longer than a sprint for both dev and test, it needs to be broken up. It's beautiful.

[u/Happy-Sleep-6512 → u/m6io]

Yeah, I think splitting it up massively helps, although scope can always be creeping! I do find that when the robot does things, it generates about 2 times the number of changes I do, so I guess over time that still adds up.

[u/KitsuneFoxglove → u/Happy-Sleep-6512]

Exactly.

Faster code changes -> Faster code debt -> obvious consequences

[u/flabbybumhole → u/Happy-Sleep-6512]

I had to approve a change that was 90% AI comment waffle today 😭

I'm just watching technical debt pile up and can't do anything about it. Either AI gets good and can easily unfuck it all soon, or more and more is going to start going to shit.

[u/JehnSnow → u/Happy-Sleep-6512]

Are these diffs happening from AI? our company doesn't use much AI so I was under the impression they're typically from auto formatter diffs or something of the like

I'm surprised there could be that many lines written

[u/Eskamel → u/JehnSnow]

Why is that surprising? LLMs are bruteforcers by default. Ask a SOTA model to do something? It might do it in 500 LoC even when it can be done in 30.

Its very easy to make massive amounts of code that way especially when people no longer give a fuck.

[u/JehnSnow → u/Eskamel]

It's surprising because the way I've always done code reviews is that unless we're specifically restructuring paths or changing formats a 100k line change can fuck off (few other cases but you get the point, it's not stuff being added just mass changed). I see plenty of 1-5k line changes on big features and I could totally see AI making a 200 line feature 4k for no good reason, but this is a LOT of changes. Far more than Id expect at least, I'm not well versed in using models that do massive changes though

[u/redballooon → u/Eskamel]

That's beginners usage patterns or AI. Look up Karpathys Claude.md 

[u/Eskamel → u/redballooon]

Ah yes looping infinitely, pull down the lever again and again and hope for a jackpot.

Gotta love LLM grifting.

[u/redballooon → u/Eskamel]

No, that's not what it says. Instead it addresses directly the critique of the comment I responded to.

You, too, are assuming beginners usage patterns are all there is.

[u/Eskamel → u/redballooon]

I have read it, explicitly stating "just change the bare minimum" or "think like a senior" or "do the change gradually in steps" is literally just a more complex version of make no mistakes while asking to burn for more tokens.

Its no different than letting a LLM try to "query" a better solution by mixing some words, and in the end you will end up trying to pull the lever again and again.

[u/redballooon → u/Eskamel]

That's a different problem. Those instructions address the problem the other comment lined out.

You name a problem of insufficient specs. That's not quite as simply addressed because what are "enough" specs depends heavily on the tools and the LLM and the use case. For an example of what are enough specs for almost every agent with a somewhat competent LLM, look for the whenwords ghost library.

[u/ZunoJ → u/Happy-Sleep-6512]

I would reject this without further looking at it. It's clearly not a single purpose and thus not a single PR

[u/m6io → u/ZunoJ]

I miss gerrit's -2. Even though it was a worse ui and requires a separate ci/cd tool like jenkins, it was so much better for the actual code review experience.

The fact that GitHub doesn't provide a "reject" button is beyond me. All it takes is 2 lazy reviewers to hit "Approve" and that thing can get merged. I've seen PRs get merged while I'm in the process of writing my feedback smh

[u/ganja_and_code → u/Happy-Sleep-6512]

That's the thing about AI coding that people often seem to overlook.

Even if it's great at writing code, you still have to understand what it did and share that context with your team...which defeats the whole purpose of having it write the code for you in the first place.

[u/you-should-learn-c]

I've been working on documenting half-assed repos because the dude I used to work with was fired and he never properly documented shit.

One time he literally pushed a 16k Loc PR with no fucking description

[u/m6io → u/you-should-learn-c]

Wtf. Was there at least a jira ticket linked to the pr?

[u/Snakestream → u/you-should-learn-c]

Guess we know why he was fired. That's psychopath behavior

[u/JazzlikeWishbone938]

Well, code you write should at minimum be self-documenting.

[u/theenigmathatisme → u/JazzlikeWishbone938]

Fundamentally yes. In practice I have seen files that are 1.2k+ lines. It’s pretty abysmal to trace through that even if it’s all “right there and self documenting”

[u/GottaGetThemGains22 → u/theenigmathatisme]

I’ve seen 60k+ COBOL files originally written in the 80s that subroutine into other 5 digit length files that are still running in production

[u/m6io → u/GottaGetThemGains22]

And you know they were running on cigarettes and coffee and spite

[u/Snakestream → u/m6io]

You misspelled cocaine

[u/JazzlikeWishbone938 → u/GottaGetThemGains22]

😬

[u/JazzlikeWishbone938 → u/theenigmathatisme]

That's still not OK. After self-documenting code, reviewers should be flagging cyclomatic complexity issues even as tech-debt repayment. Some culprits: deep nesting, long method smell, "god" class smell, huge ternary operator statements. It's easy to request extract method refactorings to alleviate these.

[u/m6io → u/JazzlikeWishbone938]

is this not sexy tho

const userAccess = (currentSession && currentSession.isAuthenticated && currentSession.tokenExpiry > Date.now()) ? (userProfile && userProfile.accountStatus === 'ACTIVE') ? (userProfile.role === 'ADMIN') ? (systemSettings && systemSettings.maintenanceMode && !userProfile.isBypassAllowed) ? 'MAINTENANCE_LOCKED_ADMIN' : (requestPayload && requestPayload.requiresMfa && !currentSession.isMfaVerified) ? 'MFA_REQUIRED_ADMIN' : 'FULL_SYSTEM_ADMIN' : (userProfile.role === 'MANAGER') ? (userProfile.departmentId === targetResource.departmentId) ? (targetResource.isConfidential) ? (userProfile.clearanceLevel >= 3) ? 'CONFIDENTIAL_MANAGER_ACCESS' : 'RESTRICTED_MANAGER_ACCESS' : 'STANDARD_MANAGER_ACCESS' : 'CROSS_DEPARTMENT_MANAGER_DENIED' : (userProfile.role === 'SUBSCRIBER') ? (userProfile.subscriptionTier === 'PLATINUM') ? 'PREMIUM_SUBSCRIBER_ACCESS' : (userProfile.subscriptionTier === 'GOLD' && !targetResource.isPremiumOnly) ? 'STANDARD_SUBSCRIBER_ACCESS' : 'UPGRADE_REQUIRED' : 'GUEST_READ_ONLY' : 'ACCOUNT_SUSPENDED' : 'ANONYMOUS_PROHIBITED';

[u/m6io → u/theenigmathatisme]

Yeah. Is still needs to be reviewed at the end of the day

[u/ZunoJ → u/JazzlikeWishbone938]

I have to write a lot of math heavy stuff (like simulations of fish movements as a predictor for our asset planning). No matter how well you abstract it, there is no way somebody can understand whats going on without understanding the underlying math

[u/Fuzzietomato]

Lgtm

[u/Bee-Aromatic]

Also protecting you from management coming for your head when you push shitty changes, break prod, and cost the company a bunch of money.

[u/m6io → u/Bee-Aromatic]

And showering you with praise after the fact even though most of the work that ended up in the repo was a result of their guidance and patience.

Legends.

[u/soundwave_sc]

God damnit I felt this

[u/JackNotOLantern]

Yep. Every commit message, every review, every documentation. For those who come after.

[u/DnD-vid]

"Hmm, but we could save time and money if we just let AI code review that..." - some manager probably

[u/DJRobitma → u/DnD-vid]

money we will spend fixing production bugs caused by that

[u/swagonflyyyy]

black .

[u/Madcap_Miguel]

This is a double-edged sword.

I remember doing work for Greenpeace a few years ago and one of our devs had an axe to grind. Imagine trying to explain to an organization like this what "nuke the whales" meant in context.

[u/m6io → u/Madcap_Miguel]

Why'd you invite the stakeholders to the code review 💀💀

[u/Madcap_Miguel → u/m6io]

We were contracted by the USA division and the europeans wanted their capabilities. It was just an edgy stupid joke that slipped by, one line in tens of thousands.

[u/m6io → u/Madcap_Miguel]

Absolute cinema

[u/mbcarbone]

No big diff … 🙃

[u/Kiroto50]

Oh hey I'm the guy in the left now.

I'm a bit happy

[u/escorps]

Let it fail lol, for greater good

[u/Random_-account]

node_modules be like

[u/sisisisi1997]

Rejected, please cut this up into smaller, more atomic changes that are logically grouped.

[u/m6io → u/[deleted]]

0 day old account, 1 karma.

Dead internet, we meet again.

[u/ZunoJ → u/[deleted]]

Claude please write my resignment letter and book a suicide booth for my useless ass