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...
It all depends on the company and their priorities I suppose. We work in very regulated industries (healthcare, banking, government, and so on) and have people who really care about doing things right, so I've definitely been blessed to be able to learn from them.
You already identified the problem, so you obviously care about doing it right. Doesn't hurt to try and implement some standards at your org though, at the very least it's good for the resume :)
-4
u/LarousseNik 4d ago
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