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

171–180 of 238 posts

Re: Code Review as a Service

#171

Post by dpweb »

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.

Not exactly. imo and just a perspective, but code review is one method of KT. Sending someone a document can also be a method of KT.

Code review for KT is one thing. Code review for finding bugs is another. Code review for following style guidelines is yet another.

I would like to use a baseline style guideline for JS is anyone aware of one that isn't too huge?

Re: Code Review as a Service

#172

Post by nick007 »

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…

I am a PullRequest user/customer. I shared some of your concerns when I first heard about them, and was admittedly the least on board them trying them vs the others on my team. In short, I didn't believe that an outsider could provide adequate reviews and that, at best, an outsider would supplement our internal review process. I was wrong, and have learned several things about code review from this company.

1. Once ramped up, PullRequest provides consistent reviewers for project, even down to fairly granular sections of the codebase. i.e. we’ll get the same reviewer or sets of reviewers who review code for backend architecture changes, a different person (but consistent) who reviews security code even within a monolithic repo. These reviews feel like a real part of your team after a while, but fully focused on providing quality review.

2. It’s true that the reviews are not involved in initial planning conversations, but in practice this turns out to be moot or even a net positive. This is because they are providing a removed perspective on the review. I can think back to one time very specifically when the team planned to implement a feature in a specific way. An engineer went off and did so as had been planned by the team. An internal review from any of the original teammates who had planned the feature with him would have immediately approved the PR since it was exactly to the original spec. However, our PullRequest reviewer caught a MAJOR VULNERABILITY that the feature’s architecture had presented. Thus, a fresh set of eyes from an outsider who knows our codebase but is not involved in planning/implementation discussions was critical. IMO this is one of PullRequest’s greatest value-adds and why I will always advocate using them /other services like them no matter what team (though I don’t know of any comparable services, though I suspect more will arise and 3rd party review becomes table stakes, but that’s a different discussion).

3. The people that PullRequest gets to do reviews are top notch. In some ways, overkill from what would be required to actually develop a feature from soup to nuts, but it gives us more confidence to let junior developers run more freely on larger features knowing that they will have to pass code review from PullRequest.

Re: Code Review as a Service

#173

Post by FullyFunctional »

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…

That was my initial reaction too, but then I thought "using such a service might encourage the code-based to be external-reviewer friendly?". That is, instead of all the shared history/context that internal reviewers have, make all that as explicit as possible, ideally in the code, or at least in good code comments. Would also go a long way to avoid the "bus factor" (or more commmon: the "sudden-quit factor").

Re: Code Review as a Service

#174

Post by NateEag »

alkonaut wrote: post #98
YPCrumble wrote: post #66

Earlier quoted context omitted.

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.

I'm talking about code where thee context is donain knowdledge and architecture. Reviewing things like style, performance, security, framework best practices etc is pretty easy work and rarely the bottleneck in a team in my experience. Basically: any kind of review where you could comment on a single file only, is easy. The important and difficult part of review is "Is this the right thing to do at all? Is it impleme…

I'll argue that, in an ideal world, most of the questions you're asking here should be addressed before anyone writes code.

A basic spec, with "here's the idea", "here's how I plan to prove the concept viable", and a rough plan of "here are the software components I'll use" doesn't take long to put together, relative to actually coding the thing up.

Having that document and getting it reviewed should answer a lot of those questions before code is committed to paper.

Re: Code Review as a Service

#176

Post by NateEag »

dpweb wrote: post #171
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.

Not exactly. imo and just a perspective, but code review is one method of KT. Sending someone a document can also be a method of KT. Code review for KT is one thing. Code review for finding bugs is another. Code review for following style guidelines is yet another. I would like to use a baseline style guideline for JS is anyone aware of one that isn't too huge?

I'm a fan of using code formatters to define how you lay out code in a file.

Prettier is popular for that job:

https://prettier.io/

For detecting functional/idiomatic/behavioral issues, ESLint is my go-to:

https://eslint.org/

This shows my bias for automation over human enforcement.

Re: Code Review as a Service

#177

Post by thewebcount »

tyler_mann wrote: post #70
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…

Disclaimer, I'm the CTO @ PullRequest. One thing to note about our service is that we are not trying to replace your code review process if it is already working well and we strongly agree that knowledge transfer is a very important part of code review. ( We actually have code review metrics as well that help encourage and reward your internal code review process. ) However what we do believe and see on a daily basis…

> Our reviewers are all highly qualified, many are maintainers of popular open source projects or work at top tech companies.

Isn’t that potentially a huge problem? What if your reviewers work for my company’s competitors? I don’t want them seeing our code base. Do you have any methods to ensure that doesn’t happen?

Re: Code Review as a Service

#179

Post by avip »

dnautics wrote: post #141
tyler_mann wrote: post #70

Earlier quoted context omitted.

Disclaimer, I'm the CTO @ PullRequest. One thing to note about our service is that we are not trying to replace your code review process if it is already working well and we strongly agree that knowledge transfer is a very important part of code review. ( We actually have code review metrics as well that help encourage and reward your internal code review process. ) However what we do believe and see on a daily basis…

I wonder if, instead of just incremental code reviews, there would also be a way to get a 3rd party review our huge codebase and flag issues (architectural, real legibility -- not just "CC measures") to be dealt with. Then you could keep track of them and burn them down as part of "killing technical debt" goals.

I'd pay $$ just to have someone reviewing our Raman bowel of a codebase and documenting it.

Re: Code Review as a Service

#180

Post by Rhodee »

:wave: Pull Request reviewer here with over 1000 reviews.

I've reviewed code across languages, team size and maturity. A good static analysis tool is not code review. The word 'context' came up 37 times in this thread (by the time I hit submit) and it is worth digging in to how I build and maintain context with teams. First up, it is my responsibility to uphold, not define a team's best current practices. If you believe a linter and static analysis tools are best current practices, you probably do not need code review. The code review process is an exercise to affirm how well a pull request meets the expectations (context) of a team and suggest remediation where appropriate.

I assume 'context' used in the threads to mean "how we do things here". As a reviewer, I conduct review understanding the problems the team wants its code reviews to optimize for or to avoid. This is due to the people creating the platform. It's a solid platform for reviewers to get things done. Every line of contributed code I review is done with an eye towards ensuring it affirms a team's stated objectives. If those objectives somehow falls outside of what I know to be true from my experiences as a professional and best current practices (BCP), I am empowered to engage the team.

I've had teams request to never provide guidance for coding style issues. Others want to know if how they modeled a React component tree could be improved. My success on the platform relies on always building context. Without that rapport it limits the depth of the review. Because code reviews are interactive, they tend to get better over time. When things go well, the relationship between team and reviewer is seeking a pareto optimum between the pull request and the team's best current practices. When needs change I adapt my reviews. It is the same treatment if you're a one person shop, SME or a listed company.

k thx bye

Post reply on HN