Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Code review can be better

tigerbeetle.com

Post reply on HN
Print view View original

51–60 of 253 posts

Re: Code review can be better

#51

Post by jrowen »

godelski wrote: post #31

While I like the post and agree with everything the author talked about I find that this is not my problem. Despite having a similar workflow (classic vim user). The problem I have and I think a lot of others have too is that review just doesn't actually exist. LGTMs are not reviews, yet so common. I'm not sure there's even a tech solution to this class of problems and it is down to culture. LGTMs exist because it sa…

Part of the problem is that businesses treat reviewing as a second class citizen. It's not "actual work" so shouldn't be given preference, which excuses the LGTM style reviews.

I've been out of the industry for a while but I felt this way years ago. As long as everybody on the team has coding tasks, their review tasks will be deprioritized. I think the solution is to make Code Reviewer a job and hire and pay for it, and if it's that valuable the industry will catch on.

I would guess that testing/QA followed a similar trajectory where it had to be explicitly invested in and made into a job to compete for or it wouldn't happen.

Re: Code review can be better

#53

Post by echelon »

Areibman wrote: post #46

The biggest grip I have with Github is the app is painfully slow. And by slow, I mean browser tab might freeze level slow. Shockingly, the best code review tool I've ever used was Azure DevOps.

> The biggest grip I have with Github is the app is painfully slow. And by slow, I mean browser tab might freeze level slow.

Javascript at scale combined with teams that have to move fast and ship features is a recipe for this.

At least it's not Atlassian.

Re: Code review can be better

#54

Post by kubectl_h »

nop_slide wrote: post #40
kjgkjhfkjf wrote: post #35

If you want to remain relevant in the AI-enabled software engineering future, you MUST get very good at reviewing code that you did not write. AI can already write very good code. I have led teams of senior+ software engineers for many years. AI can write better code than most of them can at this point. Educational establishments MUST prioritize teaching code review skills, and other high-level leadership skills.

I’m considered one of the stronger code reviewers on the team, what grinds my gears is seeing large, obviously AI heavy PRs and finding a ton of dumb things wrong with them. Things like totally different patterns and even bugs. I’ve lost trust that the person putting up the PR has even self reviewed their own code and has verified it does what they intend. If you’re going to use AI you have to be even more diligent a…

Same. I work at a place that has gone pretty hard into AI coding, including onboarding managers into using it to get them into the dev lifecycle, and it definitely puts an inordinate amount of pressure on senior engineers to scrutinize PRs much more closely. This includes much more thorough reviews of tests as well since AI writes both the implementation and tests.

It's also caused an uptick in inbound to dev tooling and CI teams since AI can break things in strange ways since it lacks common sense.

Re: Code review can be better

#55

Post by godelski »

jrowen wrote: post #51
godelski wrote: post #31

While I like the post and agree with everything the author talked about I find that this is not my problem. Despite having a similar workflow (classic vim user). The problem I have and I think a lot of others have too is that review just doesn't actually exist. LGTMs are not reviews, yet so common. I'm not sure there's even a tech solution to this class of problems and it is down to culture. LGTMs exist because it sa…

Part of the problem is that businesses treat reviewing as a second class citizen. It's not "actual work" so shouldn't be given preference, which excuses the LGTM style reviews. I've been out of the industry for a while but I felt this way years ago. As long as everybody on the team has coding tasks, their review tasks will be deprioritized. I think the solution is to make Code Reviewer a job and hire and pay for it,…

I can be totally wrong, but I feel like that's a thing that sounds better on paper. I'm sure there's ways to do this correctly but every instance I've seen has created division and paid testers/QC less. Which I'd say the lower pay is a strong signal of it being considered second class. Has anyone seen this work successfully?

I also think there's benefits to review being done by devs. They're already deep in the code and review does have a side benefit of broadening that scope. Helping people know what others are doing. Can even help serve as a way to learn and improve your development. I guess the question is how valuable these things are?

Re: Code review can be better

#56

Post by shayief »

Gitpatch attempts to solve this. Supports versioned patches and patch stacks (aka stacked PRs). Also handles force-pushes in stacks correctly even without Change-IDs using heuristics based on title, author date etc. It should also be unusually fast. Disclosure: I'm the author.

I'm not convinced that review comments as commits make thing easier, but I think storing them in git in some way is a good idea (i.e. git annotations or in commit messages after merge etc)

Re: Code review can be better

#57

Post by paulddraper »

tomasreimers wrote: post #16

Just taking a step back, it is SO COOL to me to be reading about stacked pull requests on HN. When we started graphite.dev years ago that was a workflow most developers had never heard of unless they had previously been at FB / Google. Fun to see how fast code review can change over 3-4yrs :)

Dude, I love Graphite.

Best AI code review, hands down. (And I’ve tried a few.)

Re: Code review can be better

#58

Post by benreesman »

tomasreimers wrote: post #37
benreesman wrote: post #34

Earlier quoted context omitted.

I'm a pre-mercurial arcanist refugee who tends to promote Graphite in teams that are still struggling with mega-PRs and merge commits and other own goal GitHub-isms. Big fan in general even with the somewhat rocky scaling road we've been on :) And I very much appreciate both the ambition and results that come from making it interop with PRs, its a nightmare problem and its pretty damned amazing it works at all, let a…

We've talked about a "safe mode" where we initialize it similar to JJ - such that you can no longer directly run git commands without funneling them thru graphite, but which would make it bulletproof. Would that be interesting?

I think jujitsu is interesting it it's own right!

It seems non-obvious that you would have to prohibit git commands in general, they're already "buyer beware" with the current tool (and arcanist for that matter). Certainly a "strict mode" where only well-behaved trees could interact with the tool creates scope for all kinds of performance and robustness optimizations (and with reflog bisecting it could even tell you where you went off script).

I was more referring to the compromises that gt has to make to cope with arbitrary GitHub PRs seem a lot more fiddly than directly invoking git, but that's your area of expertise and my anecdote!

Broad strokes I'm excited for the inevitable decoupling of gt from GitHub per se, it was clearly existential for zero to one, but you folks are a first order surface in 2025.

Keep it up!

Re: Code review can be better

#59

Post by faangguyindia »

nop_slide wrote: post #40
kjgkjhfkjf wrote: post #35

If you want to remain relevant in the AI-enabled software engineering future, you MUST get very good at reviewing code that you did not write. AI can already write very good code. I have led teams of senior+ software engineers for many years. AI can write better code than most of them can at this point. Educational establishments MUST prioritize teaching code review skills, and other high-level leadership skills.

I’m considered one of the stronger code reviewers on the team, what grinds my gears is seeing large, obviously AI heavy PRs and finding a ton of dumb things wrong with them. Things like totally different patterns and even bugs. I’ve lost trust that the person putting up the PR has even self reviewed their own code and has verified it does what they intend. If you’re going to use AI you have to be even more diligent a…

if you are seeing that it just means they are not using the tool properly or using the wrong tool.

AI assisted commits on my team are "precise".

Post reply on HN