11–20 of 116 posts
100% agree. While you are at it, consider naming and writing your functions in such a way that doesn't require a wall of comments. Clean Code uncle Bob style.
Just an idea which I haven't personally tried: AI agents understand technical limitations, such as CI failures. Maybe make a CI job which checks that a PR has a reasonable size, and auto-reject with a polite message if it's not? Something like, "This PR size exceeds the limit of N lines that we accept for review; if you implement a big feature please consider splitting it in several smaller PRs." There are chances th…
Yeah, that's a fun way to get massive stacks of PRs that are individually incomprehensible.
Look, if you don't think code review is worthwhile, don't do it. Just give everybody unfettered permission to merge. But don't pretend to do review if you're not trying to maintain some standard of quality.
In my experience the models perform substantially worse if asked to create small PRs or commits. They lack the ability to sequence work and understand dependencies efficiently enough to manage it – it's not that they can't do small PRs, it's that doing them takes vastly more resources which then hits context limits etc. And if you want to then go back and edit a stack of commits or PRs, rebasing work into the middle,…
So, just like you said, rewrite the whole thing, THEN break it apart into bite size chunks that tell the story and feed it to others with acceptable and reasonable context.
It’s a skill that engineers need, and it pays dividends to all on the team, including you, when your coworkers ALSO start doing this back to you and you’re asked to review it.
In my experience the models perform substantially worse if asked to create small PRs or commits. They lack the ability to sequence work and understand dependencies efficiently enough to manage it – it's not that they can't do small PRs, it's that doing them takes vastly more resources which then hits context limits etc. And if you want to then go back and edit a stack of commits or PRs, rebasing work into the middle,…
I know someone working on a smaller open source who has same thing. They have considered just blocking all PRs outside known contributors because AI spam even on their tiny open source project is too much. At work, I've gotten into fights about PR approvals. If they are beyond us humans to review, screw it, remove the approver requirement and if CI passes, merge it.
Even worse: they can write tests that make incorrect behavior part of your spec.
Tests matter.
Writing tests can be hard, boring, tedious. But if anything should still be written by hand in the age of LLMs it’s the tests. If you’re not looking at the application code anymore, you should at least be going over the tests with a fine toothed comb.
100%
but also "no" is a two letter word and one of the most important and hardest parts of being a maintainer.
> If your variable is not named well and you need a comment, name your variable better. 100% agree. While you are at it, consider naming and writing your functions in such a way that doesn't require a wall of comments. Clean Code uncle Bob style.
I’ve done it myself on:
* engine definitions for complex workflows and DSLs
* heavy graph theory sections that included ASCII diagrams to clarify flow.
But those functions are probably 1 in 100 or rarer. Basically everything else is good enough with basic IDE-helping javadoc style comments at best, maybe with some input parameter clarification and business logic-clarifying 1-2 line comments sprinkled throughout.
I know someone working on a smaller open source who has same thing. They have considered just blocking all PRs outside known contributors because AI spam even on their tiny open source project is too much. At work, I've gotten into fights about PR approvals. If they are beyond us humans to review, screw it, remove the approver requirement and if CI passes, merge it.
CI by itself is not got enough because LLMs are extremely good at writing vacuous tests that don’t actually test anything but look like the test something. Even worse: they can write tests that make incorrect behavior part of your spec. Tests matter. Writing tests can be hard, boring, tedious. But if anything should still be written by hand in the age of LLMs it’s the tests. If you’re not looking at the application c…
Earlier quoted context omitted.
Yeah, that's a fun way to get massive stacks of PRs that are individually incomprehensible.
Then reject them for being incomprehensible? Look, if you don't think code review is worthwhile, don't do it. Just give everybody unfettered permission to merge. But don't pretend to do review if you're not trying to maintain some standard of quality.
A cap on PR size isn't inherently going to make an LLM do a good job of segmenting PRs. It requires careful prompting or manual action, the kind of effort typically exerted by people who already cared enough not to hit such a cap. You may as well just ditch the cap, to save yourself from having to reject a series of PRs rather than just the one.
In my experience the models perform substantially worse if asked to create small PRs or commits. They lack the ability to sequence work and understand dependencies efficiently enough to manage it – it's not that they can't do small PRs, it's that doing them takes vastly more resources which then hits context limits etc. And if you want to then go back and edit a stack of commits or PRs, rebasing work into the middle,…
I mean, sometimes I don’t know how I want to write something until I’m finished. Huge refactors are often like this. So, just like you said, rewrite the whole thing, THEN break it apart into bite size chunks that tell the story and feed it to others with acceptable and reasonable context. It’s a skill that engineers need, and it pays dividends to all on the team, including you, when your coworkers ALSO start doing th…
This is knowledge that goes back to the beginning of software development - "Plan to throw [version] one away".
I think this could potentially become a good practice. LLMs make it so easy and cheap to just get it working and build that v1. Then you can play around with it and see if works and read the code about what could be better. Throw away the LLM generated version and now this is the part where human expertise comes in. Based on what you've learned from the v1, now guide the LLM more closely about how to write the thing and help guide it so that making small PRs that are easily reviewable and understandable are the output.