| ▲ | ivanjermakov a day ago |
| Code review process is the worst part of LLM-assisted SDLC at the moment, probably because nobody knows how to do it well yet. |
|
| ▲ | Buttons840 a day ago | parent | next [-] |
| The human experience of reviewing PRs hasn't really moved beyond "look at these 2000 lines of text and think really hard". At least for me. Am I missing something? |
| |
| ▲ | NewJazz a day ago | parent | next [-] | | I at least: Use CI results to control quality, no nitpicking style means easier review Review tests one by one, to make sure I understand what the submitter is trying to test and if they actually did so Check files off one-by-one, compartmentalizing understanding as I go Complain loudly when PRs try to change too many subsystems or different functionality, especially if less related changes are not separated into standalone commits | |
| ▲ | jeffbaumes a day ago | parent | prev | next [-] | | I often have Claude make code walks specifically to present walls of code more sanely, add diagrams, bring in code context outside the diff, etc. This is one manifestation I made but I think there are many ways people are solving this (or roll your own tool!) https://github.com/jeffbaumes/code-walk | | |
| ▲ | Buttons840 a day ago | parent [-] | | This is a great idea. Combine this with a tool that allows you to mark the lines of code you have reviewed and understand, and you're on your way to systematically working through the PR. |
| |
| ▲ | jayknight a day ago | parent | prev [-] | | I mean if it's a code review, you gotta review the code. | | |
| ▲ | johannes1234321 a day ago | parent [-] | | Yes, but code can be presented in different ways. This depends of course to a degree on the submitter (large changeset touching a lot of files or smaller change sets building upon each other?) but also could be presented differently (highlighting changes to data structures, identifying core parts of code and helper routines - lots of room for imagination) Also mind: one doesn't only have to review changes code, but also review what hasn't been changed (are preconditions of other code still met?) | | |
| ▲ | karakrakow a day ago | parent [-] | | I've been trying to auto generate diagrams on PRs I'm reviewing. Flows changes via sequence diagrams, state changes and schema changes, post it to the PR with a skill. Then look at the relevant code based on the diagrams. Then \code-reviews. Moderately helpful. YMMV |
|
|
|
|
| ▲ | criley2 a day ago | parent | prev | next [-] |
| Code review is a problem only because it is a backstop for everything else. We skip architecture review, we skip prototyping, we one-shot a result in isolation from our team and then expect a human at the end of the line to do our job for us?? Code review is great if the code isn't a surprise, the architecture was already agreed on, the AI reviewers are already happy, and you just need a final pass. Code review was already terrible before AI if you were slopping down weeks of work on a poor teammate to sift through. I tell the younger engineers at my company the same thing today I said 5 and 10 years ago. Make the review easy for another engineer to do it, and they'll do it. Make it hard on them, and you won't ship shit. |
| |
| ▲ | gertlex a day ago | parent | next [-] | | I'm fully with you on this philosophy. But the "make review easy" part is one where I'm failing (a bit) to keep up, I feel. Also, what does "a final pass" look like to you? E.g. if a PR is 3000 lines... I'd guess one approach is enforce smaller PRs. I've opened a few, looked at the line count, and closed them, to be redone/broken into stacked PRs. If I'm reviewing a PR that I'm familiar with the plan/work on, my "a final pass" is often just "read and suggest touch-ups of the docs". But that's really just the first step of a "full review" too, so it skips other stuff, and I know that isn't a proper review in the back of my head, but on to the next thing! :| (Because of several factors including joining a much less mature org git-wise this year, I'm full of internal questions/dilemmas around PRs that I'm working on feasible solves for) | |
| ▲ | ivanjermakov 19 hours ago | parent | prev [-] | | Code review complexity is always proportional to the associated work unit's complexity, not to some imaginary "keep PRs nice and small and nifty" complexity. I agree that consequences of bad SDLC ofter float up during code review, all the way up to formulating requirements and managing human resources. |
|
|
| ▲ | dirtbag__dad a day ago | parent | prev | next [-] |
| I don’t believe that nobody is doing it well. May I ask who is doing it well? Either in house or with an AI reviewer vendor. For me, what works is highly templated directory structure and rules that linters can quickly check and enforce. Every PR is a cookie cutter design filled in. There are only a handful of things you need and they’re all just CRUD at the end of the day. You can employ the same very few design patterns and primitives across a lot of your stack. Review is so fast, if you trust your CI to do its job and the developer to actually QA their own work. What isn’t working is the sheer volume of PRs. Even with CODEOWNERS directing attention to only high impact changes, it is a constant distraction. I’m not sure how to handle SOC2 where we need a human reviewer on all code. Lastly, agents shit the bed when they touch spaghetti, esp when it’s sitting next to structured patterns. IME, the best way to get around this is to just rewrite features from scratch. But in this current world of instant completion and reward, it’s been challenging to convince people to commit to more ambitious lifts. Finally, I want review more than ever. A team can’t keep up with the pace of change even if it was all perfect. I like the idea of more up front planning, but the code is ultimately what matters. |
| |
| ▲ | NewJazz a day ago | parent | next [-] | | There are only a handful of things you need and they’re all just CRUD Idk why everyone says this. Yeah crud is tedious and common. No it is not the majority of my time spent, implementing, fixing, or reviewing. | | |
| ▲ | dirtbag__dad 10 hours ago | parent [-] | | what do you work on that isn’t CRUD? | | |
| ▲ | NewJazz 10 hours ago | parent [-] | | Well I have signed an NDA so I can't be too specific... But generally business logic, heuristics, event correlation. Essentially applying institutional knowledge and rules to stabdardized processes. Either that or integration between different systems in nontypical ways. |
|
| |
| ▲ | eddythompson80 a day ago | parent | prev [-] | | I’m waiting to see someone who has any sort of reasonable way, but I have yet to see one. Code review was always a difficult part of the job. However, it was self regulating because the people who would do very large code dumps frequently were very very few high throughput individuals and they were either legitimately good and knew what they were doing, so you trusted them, or were quickly told to not do that. The process had natural breaks on it, and particularly large PR would be expected to take few days or week(s) to review. This all has changed. I’ve only seen gaslighting (in the form of “of course we expect you to read the code LLMs is generating”), lying along the same lines, or the realistic people who acknowledge they don’t read any of it. The former camp is mostly traditional engineers, and the later is mostly new or traditional non-engineering roles like recent product management coding converts. I think it would help everyone if we just normalize the fact that no one if reading code much anymore. It’s just not feasible. Write a bunch of creative reviewing bots or SKILLs and just leave it at that. It’s not a great answer, but like I said I haven’t seen any alternatives. > if you trust your CI to do its job and the developer to actually QA their own work. I trust the CI, but the CI is doing what the CI is written to do. I don’t know how you can achieve the second part in a large team/corporate setting or an open source setting. | | |
| ▲ | ivanjermakov 18 hours ago | parent | next [-] | | > of course we expect you to read the code LLMs is generating Yes! I think a lot of struggle comes from not having the same reliance on LLMs across the team. If developer A is not opening IDE and not reading the code to do work and developer B does not use LLMs, types code letter by letter and reads every text they encounter - it will not go well for the team. I also think many devs rely on LLMs more than they're willing to admit to the management and the rest of the team. | |
| ▲ | nkohari 18 hours ago | parent | prev [-] | | With Theseus, we've been trying to approach the problem from a couple of standpoints: 1. Code review is now less about mechanical validation (which often happens in linting, AGENTS.md, etc.) and more about education and understanding what's changing
2. Engineers need a tool which allows them to focus their attention on parts of changes, without having to review everything all at once Would love to hear what you think! [0] https://theseus.dev | | |
|
|
|
| ▲ | nkohari 18 hours ago | parent | prev [-] |
| We're trying to solve this with Theseus. Would love to hear your thoughts! https://theseus.dev |
| |