| ▲ | 0xbadcafebee 2 days ago | |
Reviewing PRs should be for junior engineers, architectural changes, brand new code, or broken tests. You should not review every PR; if you do, you're only doing it out of habit, not because it's necessary. PRs come originally from the idea that there's an outsider trying to merge code into somebody's open source project, and the Benevolent Dictator wants to make sure it's done right. If you work on a corporate SWEng team, this is a completely different paradigm. You should trust your team members to write good-enough code, as long as conventions are followed, linters used, acceptance tests pass, etc. | ||
| ▲ | sensanaty 2 days ago | parent | next [-] | |
> You should trust your team members to write good-enough code... That's the thing, I trust my teammate, I absolutely do not trust any LLM blindly. So if I were to receive 100 PRs a week and they were all AI-generated, I would have to check all 100 PRs unless I just didn't give a shit about the quality of the code being shit out I guess. And regardless, whether I trust my teammates or not, it's still good to have 2 eyes on code changes, even if they're simple ones. The majority of the PRs I review are indeed boring (boring is good, in this context) ones where I don't need to say anything, but everyone inevitably makes mistakes, and in my experience the biggest mistakes can be found in the simplest of PRs because people get complacent in those situations. | ||
| ▲ | iLoveOncall 2 days ago | parent | prev | next [-] | |
It seems like LLMs really have made people insane. | ||
| ▲ | blub 2 days ago | parent | prev [-] | |
(Not the OP) For many years, all the projects I’ve been in had mandatory code review, some in the form of PRs (a github fabrication), most as review requests in other tooling. This applies to everything from platform code, configuration, tooling to production software. Inside a component, we use review to share knowledge about how something was implemented and reach consensus on the implementation details. Depending on developer skill level, this catches style, design issues or even bugs. For skilled developers, it’s usually comments on code-to-architecture mismatches, understandability, etc. Sometimes not entirely objective things, that nevertheless contribute to developing and maintaining a team consensus and style. Discussions also happen outside and before review, but we’ve found reviews invaluable. If a team has yearly turnover or different skill levels (typical for most teams), not reviewing every commit is sloppy. Which has an additional meaning now with AI slop :) | ||