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

Code Review as a Service

pullrequest.com

Post reply on HN
Print view View original

161–170 of 238 posts

Re: Code Review as a Service

#161

Post by mncolinlee »

andy_ppp wrote: post #157
mncolinlee wrote: post #129

Earlier quoted context omitted.

Having done more than a few PR reviews and code security reviews for their platform as an Android/Kotlin dev, I've found that the opposite problem is more common. A lot of organizations suffer from insular thinking and their own team often comments LGTM even if there's something glaring. Writing reviews as an outsider, there's something freeing about knowing that you can review honestly and professionally and not ove…

I think the point is internal and external code reviews are two different beasts - no harm in getting an external kicking to improve the coding practices. However, with nobody having skin in the game to get external code reviews into the codebase, they will largely be ignored as “nice but we have work to do”. How could a product like this (I think I’ve seen a few) solve that human nature problem?

They're truly different beasts, but each has clear value. As but one example, I've seen outsourced apps for financial firms where there were literally hundreds of basic security flaws. Would you trust the same review process that allowed those PRs?

Re: Code Review as a Service

#162

Post by fourseventy »

YPCrumble wrote: post #66
alkonaut wrote: post #44

I'm all for this if the person reviewing my code will know the context, history and all the details and conversations we had as a team. But in order for that to work, I'd probably be taking most of this reviewer's time. And obviously in order for them to get up to speed with our practices, conventions, architecture, code style and whatnot, they'd probably need to start by doing a whole lot of development on our proje…

Code should be written to be understood without that context. If comments and documentation aren’t enough context for the code to be understood, it probably isn’t written very well.

Not very realistic for the real world

Re: Code Review as a Service

#164

Post by tantalor »

cannonpalms wrote: post #27

This entirely misses the point of code review. Say it with me: Code review is a knowledge transfer exercise. Finding bugs, security vulnerabilities, and keeping the code maintainable are merely side effects that we appreciate along the way. The primary purpose of code review is increasing the bus factor of the given piece of code and facilitating organic knowledge transfer. That's it.

You're thinking of pair programming. That's different.

Re: Code Review as a Service

#166

Post by ryanbrunner »

mncolinlee wrote: post #129
alkonaut wrote: post #44

I'm all for this if the person reviewing my code will know the context, history and all the details and conversations we had as a team. But in order for that to work, I'd probably be taking most of this reviewer's time. And obviously in order for them to get up to speed with our practices, conventions, architecture, code style and whatnot, they'd probably need to start by doing a whole lot of development on our proje…

Having done more than a few PR reviews and code security reviews for their platform as an Android/Kotlin dev, I've found that the opposite problem is more common. A lot of organizations suffer from insular thinking and their own team often comments LGTM even if there's something glaring. Writing reviews as an outsider, there's something freeing about knowing that you can review honestly and professionally and not ove…

I definitely get what you're saying here, but I think if I was given the choice between an internal reviewer that might glaze over some bad practices, or an external reviewer who will miss stuff like "oh be careful calling that code, there's gotcha X, Y, and Z that you need to think about", I'd take the former every time.

Re: Code Review as a Service

#167

Post by Jach »

cannonpalms wrote: post #27

This entirely misses the point of code review. Say it with me: Code review is a knowledge transfer exercise. Finding bugs, security vulnerabilities, and keeping the code maintainable are merely side effects that we appreciate along the way. The primary purpose of code review is increasing the bus factor of the given piece of code and facilitating organic knowledge transfer. That's it.

The case for code review is fundamentally economical: it saves the organization money by finding costly issues earlier when they are cheaper to fix without imposing a costlier burden for the review process itself.

As generic "knowledge transfer", or even increasing the bus factor, I would disagree. The best knowledge transfer experiences I've had have been dedicated meetings/workgroups dedicated to that purpose, and they tend to encompass larger scopes than a single commit/pull request. I've also seen the difference in devs who contribute to an area having previously only been reviewers of code in that area, vs. actually having a knowledge transfer session with that area's lead beforehand, and in the latter case they are more effective (actually even if they had never reviewed code in that area before, as was the case with interns or new hires). To me knowledge transfer is the nice possible side effect, but not the primary purpose.

I'll leave a third opinion from here https://static1.smartbear.co/smartbear/media/pdfs/best-kept-... which lists some direct and indirect benefits:

    Few developers and software project managers would argue that these are direct benefits of conducting code reviews:
    
    • Improved code quality
    • Fewer defects in code
    • Improved communication about code content
    • Education of junior programmers
    
    And these indirect benefits are byproducts of code review:
    • Shorter development/test cycles
    • Reduced impact on technical support
    • More customer satisfaction
    • More maintainable code

Re: Code Review as a Service

#168

Post by watwut »

nerdjon wrote: post #74
cannonpalms wrote: post #27

This entirely misses the point of code review. Say it with me: Code review is a knowledge transfer exercise. Finding bugs, security vulnerabilities, and keeping the code maintainable are merely side effects that we appreciate along the way. The primary purpose of code review is increasing the bus factor of the given piece of code and facilitating organic knowledge transfer. That's it.

Personally I work on a 2 person team, the vast majority of my reviews either have no changes or "hey maybe this thing should be named something else so its more consistent". For us the peer review is almost completely for knowledge transfer. Sure we know what each other is working on, but we still have to maintain each others code if something goes wrong and the other is not available. So I agree with this 100%. Even…

We do have detailed code reviews. People will complain about lines you write and expect a lot of consistency.

Simultaneously, architecture is complete mess, because architecture is much harder to see in code review.

Re: Code Review as a Service

#169

Post by vinceguidry »

skeeter2020 wrote: post #146
vinceguidry wrote: post #51

Earlier quoted context omitted.

I mentioned this idea to a very knowledgeable enterprise architect. I was mentioning how much I'd enjoyed being able to use the code review process in this way. His reply: As good of an idea that is, it breaks Agile. So long as the code meets the biz-provided spec, it must be accepted, and if there are other concerns with the code, to make a tech debt ticket to address it at a later date. This, of course, horrified m…

>> it breaks Agile It must be "nice" working with an enterprise architect who views agile as a strict, narrowly defined thing.

He's really sharp and has keen insight into how enterprise business works. His advice saves us a lot of time and money.

Re: Code Review as a Service

#170

Post by Jach »

nathanielarmer wrote: post #154

I'm a reviewer on PullRequest, and thought I'd share some perspective. Happy to answer any questions in comments. Background - I've built and led engineering teams at multiple fast-growing startups. In doing so I've seen the incredible value PR's can provide, but also the huge cost of them on small teams. I review on PullRequest part-time as I work full-time building a startup. Many of the critics here are right. The…

Here's a question: do you get pushback about "best practices"? (Sometimes really just "standard practices".) If so, which ones in particular? And do you actually continue trying to explain why it's a good idea, or just throw up your hands with a feeling of "I gave my advice, they can ignore it at their peril".

Having actually convinced people of the value of certain practices (like favoring fast focused unit tests over slow broad integration tests that happen to require the full system to be running), which leads to smoother and more succinct reviews in the future -- like "please add tests" doesn't even need to be said if the developer values them and already wrote some -- I don't know if I'd put up with the stress of having to fight the same battles week after week with a new group. Though I suppose it's possible that you might get good at convincing people of a certain thing, and the ease of convincing can reveal whether the thing is really closer to "best" or just "standard".

Post reply on HN