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

The Theatre of Pull Requests and Code Review

meks.quest

Post reply on HN
Print view View original

51–60 of 431 posts

Re: The Theatre of Pull Requests and Code Review

#51

Post by gjgtcbkj »

surgical_fire wrote: post #16

Can't relate. I take code reviews as possibly the most important part of my job as a developer. Suggesting extra tests, thinking about unintended side effects, and yes, aiming for consistency and readability, without being picky on style choices. I trust my colleagues to do the same (and they often do). I can't imagine working in an environment where this is a theater.

It sounds like a good job where the most important part is finding other people’s mistakes.

Though I do appreciate the shoutout to adding tests in CR. But returning a PR solely because it doesn’t have tests, is effective, but a little performative too. It kind of like publicly executing someone, theirs gotta be some performance for it to be a deterrent. If something doesn’t have tests my review is going to be a very short performance where I pretend read the rest of the code. Then immediately send it back.

Re: The Theatre of Pull Requests and Code Review

#52

Post by osigurdson »

chrisweekly wrote: post #26
2sk21 wrote: post #19

Reviewing someone else's large pull request is like having a second task in parallel with what you are working on yourself!

It's not "like" another task, it IS another task!

Yeah but it is just a quick look, "yep", "yep", "oh what about this"?, "wow we dodged a bullet there". Its like self managed error correction that the collective does on its own. Fast, simple and produces good results. The less software you write the more this resonates.

Re: The Theatre of Pull Requests and Code Review

#53

Post by surgical_fire »

gjgtcbkj wrote: post #51
surgical_fire wrote: post #16

Can't relate. I take code reviews as possibly the most important part of my job as a developer. Suggesting extra tests, thinking about unintended side effects, and yes, aiming for consistency and readability, without being picky on style choices. I trust my colleagues to do the same (and they often do). I can't imagine working in an environment where this is a theater.

It sounds like a good job where the most important part is finding other people’s mistakes. Though I do appreciate the shoutout to adding tests in CR. But returning a PR solely because it doesn’t have tests, is effective, but a little performative too. It kind of like publicly executing someone, theirs gotta be some performance for it to be a deterrent. If something doesn’t have tests my review is going to be a very…

> It sounds like a good job where the most important part is finding other people’s mistakes.

And reviews are not that. Systems are complex, and having a mental model of complex systems is difficult. Everyone has blind spots. A fresh pair of eyes can often spot what who was coding would not.

> But returning a PR solely because it doesn’t have tests, is effective, but a little performative too.

And this is not what I said. I spoke of suggesting extra tests. A scenario that wasn't covered, for example.

Re: The Theatre of Pull Requests and Code Review

#54

Post by gjgtcbkj »

octo888 wrote: post #30

What I've often found is that people only really accept feedback from the Tech Lead, and peers are dismissed (not outright and not obviously - kind of sealioning etc). Peer-to-peer code reviews are another instance implementing a thing that pretends hierarchy does not exist. You can only get basic tweaks accepted. The sunk-cost fallacy is a huge force. Maybe I've only worked at crappy places

Allen Iverson got criticized by the media for “letting down his teammates” for skipping a few practices. He famously said “We’re talking about practice, practice! not the game! How is MY going to practice gonna make THEM better?!” He got flack for those comments, but what he said was accurate. If we’re talking about outcomes you’re beholden to the person who is seen as the difference maker, all the teamwork in the world isn’t going to improve an individual’s the lack of ability.

Re: The Theatre of Pull Requests and Code Review

#55

Post by stack_framer »

My manager recently told our team that "AI usage" would be added to our engineering competencies, and we would all be expected to "use AI more."

When I said my top preference for AI usage, by far, would be to eliminate human code reviews, the response was basically, "Oh, not like that."

Re: The Theatre of Pull Requests and Code Review

#56

Post by Entelligence25 »

jvanderbot wrote: post #5

PR review is probably at least a little performative. But I trust my colleagues to do good reviews when I ask them to, and to ignore my PRs when I don't. That's kind of the way we all want it. I regularly ask for a review of specific changes by tagging them in a comment on the lines in question, with a description of the implications and a direct question that they can answer. This, "throw the code at the wall for in…

[flagged]

Re: The Theatre of Pull Requests and Code Review

#57

Post by CuriouslyC »

davedx wrote: post #41

It's a very common refrain but I don't really agree with it: "How do you create a PR that can be reviewed in 5-10 minutes? By reducing the scope. A full feature should often be multiple PRs. A good rule of thumb is 300 lines of code changes - once you get above 500 lines, you're entering unreviewable territory." The problem with doing this is if you're building something a lot bigger and more complex than 500 lines o…

100%. I think the right answer is to break features into atomic commits, but keep PRs at the feature level. This reduces the PR friction, while letting reviewers easily view change sets for specific features, and if a specific feature needs a patch you don't need to do any rebase gymnastics, just add a patch commit.

AI agents like frequent checkpoints because the git diff is like a form of working memory for a task, and it makes it easy to revert bad approaches. Agents can do this automatically so there isn't much of an excuse not to do it, but it does require some organization of work before prompting.

Re: The Theatre of Pull Requests and Code Review

#58

Post by eterm »

2sk21 wrote: post #19

Reviewing someone else's large pull request is like having a second task in parallel with what you are working on yourself!

So don't do it in parallel.

Completely park other tasks, spend time on the review and record that time appropriately.

There's nothing wrong with saying you spent the previous day doing a large review. It's a part of the job, it is "what you're working on".

Re: The Theatre of Pull Requests and Code Review

#59

Post by Entelligence25 »

jvanderbot wrote: post #5

PR review is probably at least a little performative. But I trust my colleagues to do good reviews when I ask them to, and to ignore my PRs when I don't. That's kind of the way we all want it. I regularly ask for a review of specific changes by tagging them in a comment on the lines in question, with a description of the implications and a direct question that they can answer. This, "throw the code at the wall for in…

[flagged]

Re: The Theatre of Pull Requests and Code Review

#60

Post by CuriouslyC »

stack_framer wrote: post #55

My manager recently told our team that "AI usage" would be added to our engineering competencies, and we would all be expected to "use AI more." When I said my top preference for AI usage, by far, would be to eliminate human code reviews, the response was basically, "Oh, not like that."

Don't worry, if you actually increase AI usage your team will be forced to automate code review, either explicitly or implicitly.
Post reply on HN