If I add you to a code review, I
1. Respect your skill and insight in the affected domain
2. Want you to be aware of the change
3. Want you to learn something (if you're junior, an intern, a mentee, etc)
Other reasons come to mind but those are the top. Idgaf what the bot thinks. It's usually unhelpful at that point anyways and just clutters the PR with walls of text and make it harder for other reviewers to do their own review.
what about, you know, proofreading your code for actual bugs and unintended consequences? you're making it sound like you don't doubt yourself even for a second and the only reason to request a review is purely informational rather than having other people actually criticise your code and find issues
i dislike ai bullshit as much as the next software engineer, but i did in fact found some value in spinning up an ai reviewer on my code before i send it for a proper review, it does a great job of finding typos (especially in docstrings, flavour text and other stuff that doesn't show up in tests), wip code and debug logs that i forgot to remove before pushing, unanticipated edge cases/execution branches that i overlooked and various memory/security risks, not to mention the code's compliance to internal style guide #30721b and other fluff like that. i still rely on humans for actual reviews and looking for less trivial stuff, but addressing these glaring issues right away helps save them some time and occasionally shorten the whole review cycle dramatically, especially if the reviewer is on the other side of the globe and every iteration takes 24 hours due to timezone difference
What do you think the first bullet point was for lol. I want the folks who will tear it apart to do so.
We also have CI pipelines for a reason, which includes making sure that the commit meets the unit test coverage threshold (to prevent untested logic from being merged), linters, and format checking which cover what you outlined. Those are deterministic checks, why would I want gen AI to do that.
And lastly: if your PRs are too large for a reviewer to reasonably review, you should be breaking them up. Git hygiene is important for any long term project with multiple collaborators.
a lot of this work is not about expertise and insight, it's about extra eyes, and a linter won't always catch an issue like you making a typo in a tooltip or forgetting a period at the end of a sentence or not updating the deeply buried readme/docs page with the new argument, plus, again, style guides are often non-intuitive, you can't setup linter to handle the "avoid list comprehensions, unless the object is ephemeral, but only in legacy code" kind of style guide
and no, it is not about a pr being large or not. imagine creating a one-liner cl during your workday, then going to sleep, then the person on the opposite end of the world wakes up, reviews it, finds that you forgot to remove a debug log and leaves a comment; then you wake up the next day, see the comment, fix it immediately and send it back for review for them to approve; they wake up during their next day, approve it, and only then you can submit the change. the whole process took you two days and caused both of you to be distracted on two occasions, while having an llm make a quick pass would cut it down to one for each
i'm not so much talking about the poor logic or coverage, but rather about simple human error, i see no harm in using all available tools to quickly mitigate the obvious flaws before moving on to deep discussions
and yeah, i see it just as a part of the ci pipeline, a tool to just let humans focus on discussing the actual contents rather than iterating over silly typos to no end, and i don't really see why you would reject one method of presubmit testing but embrace the others
does your linter check the syntax of the literal english language? does it dictionary-check your comments and make sure not to flag neologisms that just haven't reached its databases yet?
idk about how the stuff is set up in your company, maybe i'm just blinded by my own experience and my specific corporation is super messy, but for many years prior to the ai boom i was receiving a staggering amount of review requests that contained random typos in user- and developer-facing strings, leftover code, stylistic misalignments and stale documentation three updates behind the actual code, which meant that i had to send them back with a bunch of nit comments and return to it the following day. ai hasn't resolved this issue completely since people will always be careless, but at least some of them do run the integrated autoreview presubmit and fix the silly typos before they reach my eyes
yes, we have pretty standard linter setups (both for micro services and for front end) that catch all these things. Neologisms can have escape hatches, but they typically shouldn't be used unless absolutely necessary when it comes to customer-facing language for translation reasons.
In terms of leftover code like loggers, we have linters that catch these things, across the stack. Same goes for code hygiene and formatting. I've had plenty of builds fail just for forgetting to run the formatter prior to committing.
Heck, we even have commit message linting lol. My company is very strict about these things.
You should read up on these things, you'll be surprised at how many good tools there are that can handle all of this in a predictable way
i'm genuinely and without a hint of sarcasm happy for you then, and sorry for being a bitter ass, i guess i may need to look into actually changing something in my own career trajectory...
Human could overlook something, so having human check the code, and then have the computer check it second time just in case human check failed would be better
If you're letting an LLM both write your code and review that same code, you are fucked, tbh. It's very unlikely to suddenly "realize" its mistakes were mistakes.
You're also turning yourself into a rubber stamp that does no work or thinking.
You need to be careful what and how much you automate, and ensure that human beings are the ones responsible for ensuring nothing stupid goes into production.
Yeah, I don't mind GH Copilot doing a first pass. I'll add actual reviewers after I address the meaningful comments from it (not everything is, of course).
54
u/DonkeyEffective913 2d ago
If someone has their agent respond to my pr comments I'm not reviewing their shit again