6 ms·

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 t

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 project first. At least several months. They could probably not do anyone elses code review then. So we'd have to pay them... one full time salary to be this coder-and-reviewer type person. I wonder what this service should be called. I think I might be onto something here. "Full time developers as a service"

andreyk 5y ago
While I agree in general, some forms of code reviewing require less context than others. They say "We review within your tools to catch security threats, stop crashes, and fix performance issues before they reach production.", and it does seem to me that these things are less a matter of style/context than just noticing potential issues. Not sure though, I had the exact same thoughts as you when first seeing this. Edit: to be clear, I personally don't see the value here, just playing devil's advocate.
- jermaustin1 5y ago
  But almost any good SA can catch all of those issues, a person doesn't need to look through the code for that.
  - senko 5y ago
    This is very close to saying Halting problem is not, in fact, a problem: https://brilliant.org/wiki/halting-problem/ https://brilliant.org/wiki/halting-problem/
    - jermaustin1 5y ago
      Not at all, it is saying that if the only things this service is providing is a human doing SA (since they dont have any deep-knowledge of the codebase), then you could just use an SA. Code review in the real world misses the Halting problem, the only way you can really see it is if you know the codebase well enough to SEE it, and any 3rd party that is given a pull request to complete in a timely manner will not have time to fully learn your code base or even the modules you are submitting.
    - adgjlsfhk1 5y ago
      This isn't quite true because in most real world scenarios, one of the requirements for code to be correct is that it provably halts in a fairly limited amount of time.
    - reuben364 5y ago
      A human being doesn't solve the halting problem either.
    - samatman 5y ago
      Ironically, it very seldom is. It's Rice's Theorem which tends to get me in trouble!
- HPsquared 5y ago
  What will actually happen, is people will use these kinds of "context-free code review" services, then say a code review has been done. Technically correct in a narrow way, but not what most people would expect.
- malux85 5y ago
  I suspect that there’s only a tiny, tiny fraction of issues that are above the complexity that a linter and test framework can’t identify, but are below the complexity that requires deep codebase knowledge, and I would say that the overwhelming majority of value of code reviews is inside the issues that require deep codebase knowledge. Maybe this service could be good to point out superficial security bugs though, but the infrequency of these coupled with the effort of external human engagement I think would be a barrier. Also a few times in my career I’ve seen a team with no senior devs on it, they usually fail due to inexperience, maybe this service could help a team like this - who are producing a lot of really obvious mistakes …
- alkonaut 5y ago
  Code the falls between between "I need an experienced team member to see the issue" and "A few static analysis tools would no doubt have seen it" must be vanishingly small. For specific areas, I can see myself paying for external code review. For example some open source library authors sell it as a service, so they can review the specific parts of the code where the library is used. Rob Menschings FireGiant is one such example. Security reviews (Audits) I can also see a good use for. The current practice here is already to have external non-domain-experts review the code. So making that simpler or more frequent is a net win. The reviews I see little use for would be normal feature code, (pull request reviews) which need a lot of context and domain knowledge to even begin to review. Not audits, or reviews of specific areas or aspects of the code.
YPCrumble 5y ago
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.
- blacklion 5y ago
  It is very bold statement. Code models some real-world entities (domain). Code (completely with comments and documentation) cannot and should not document fully domain. It is context, which is needed to understand code. Yes, simple CRUD application can have all context encapsulated, but what's about some code which models, say, some aspect of chemistry? Should this code have enough context which includes several post-grad university courses? Or «simpler» example from my current $Job: we have a lot of code to build some models of derivative stock exchange instruments (options, futures, etc). Enough context for this code is, like, full shelf of 1000 page books. Good luck to review this code for everything but off-by-one errors if you don't work in this area for 5+ years.
  - coryrc 5y ago
    Your example is, IMO, the exact use of this service. If you're a chemistry expert, you're probably not a coding one. These reviewers will ensure your code is testable and likely to do what you hope it does, in a way where your fellow experts can read and write their own automated proofs (tests).
- alkonaut 5y ago
  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 implemented using the right approach to begin with? Do we have other functionality that already does this? Does that other functionality use the same approach or is there good reason for this being different? Does this follow the business logic properly or are there any signs of misunderstanding the requirements? Are the requirements sensible?"
tyler_mann 5y ago
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 is degree that we help supplement the process and help catch many issues as well as inject a unique perspective. Our reviewers are all highly qualified, many are maintainers of popular open source projects or work at top tech companies. Our reviewers also gain context over time similar to a new senior engineer on your team. Reviewers also can share notes with each other to build up a corpus of information for your project over time.
- Remed 5y ago
  I wanted to join as a reviewer, but you don't accept reviewers from outside the US. Is this something what you are working on and has a chance to change in the foreseeable future? I work for a public US tech company but I'm located in Europe.
- winterplace 5y ago
  Hi Tyler. Is there a reason why PullRequest Reviewers from Canada are also included as an allowed country? Because that country has good rule of law?
- gwbas1c 5y ago
  For the past year, I've been working for a company where there are a lot of extreme novice mistakes in the codebase. Even though they reviewed their code, a novice developer reviewing another novice developer aren't going to catch things that are obvious to a developer with 5+ years experience. IMO: Target shops where they just don't have the expertise on-hand to do thorough code reviews. Don't waste time trying to convince a team full of experts with deep domain knowledge that they need you. (They probably don't.) We also have a "problem" where there are some components that are a different language than what most of us are experts in, so they end up being developed by a solo developer. When we need to jump in, as we learn the codebase, we also see novice mistakes that are very hard to fix, because we just don't have many years of experience in that language / platform. Thus, IMO, on your website, list out situations where shops will clearly identify a need for your service. (Team full of novices, solo developers, team members quit.) Don't go trying to convince "everyone" that they need you.
kevingibbon 5y ago
My company has used them before. I thought the same before using them. However I was surprised the level of talent they have. For a super senior dev the context that you need for most projects isn’t as much as you think. It was a net positive for us. Sped up our developer process given its so hard to hire senior devs right now. They also have domain experts. So say you are using some tech your team isn’t as familiar in. Great to get some extra eyes of that code to check for security issues, potential computation issues etc. Also they are much more broader than their company name suggests. Think of them as developers as a service. In this hiring environment it’s much needed.
- walkingriver 5y ago
  I'm one of those reviewers, and I agree with you about the talent. Not speaking about myself, but the people I've gotten to know and also the ones I have referred. Each PR gets two reviewers from pullrequest.com, and we get to see each others' comments. One will catch stuff the other misses, and we usually support each other. It's most fascinating when we disagree on something, which so far has always led to a high-quality discussion between the engineers and the reviewers. I've been working with them for most of 2021, and I can honestly say I'm impressed with the review comments I have seen. It's been nothing but respectful and professional. As a plus, it's made me a better code reviewer at my day job.
  - winterplace 5y ago
    Hi. I have a question. Do you have an email address or a method of contact?
  - gavinray 5y ago
    You do this on the side of your dayjob for extra income?
    - mncolinlee 5y ago
      I have certainly done this. PullRequest bought the Moonlight developer gig platform. A lot of full-time developers from Moonlight also took on gigs from PullRequest as they've been using the platform to sell their service. I'd guess most reviewers have other jobs or contracts.
koblas 5y ago
As somebody who's done a bunch of reviews on the platform you realize a bunch of things about development that we don't want to admit. When I review code for my own team (for my day job), there are many times where internal pressures on my time will make me prematurely stamp LGTM on one of my co-workers PRs that I trust. When I'm doing reviews on PullRequest I remove the "trust" marker in my review along with this strange thing called "economic compensation" where I'm paid to spend time working on it, rather than having somebody ding me for not completing some other task that needs to ship this week so I spend more time reviewing the details. I think there are a few levels of code reviews that should be in place. * Automated checks eslint/typescript as examples, you would be surprised at how many companies don't have this! * Best practices -- react hooks, golang interfaces, calling conventions... * Testing -- are the tests structure to test good and bad, are they brittle? * Security -- Did you build the right IAM role in terraform, did somebody just checkin their GITHUB key (ok, that should be automated). * Performance -- Is that Promise.all going to dispatch 1000 calls in parallel. * Architecture and inter dependancies, there is a limit for a 3rd party here. Now if you're the Lead/Architect on a project and at a minimum you can outsource the Best practices / Testing portion of the pull request to a 3rd party you now can focus on the architectural dependancies that are really what you care about. As an architect you can easily provide reviewer notes to the person doing the review that you are interested in focusing on specific areas of improvement across your team. Giving you the coverage to focus on the high level issues and inter dependancies while not focusing on variable names or test coverage. Your time is valuable, you should spend it on character development not on the punctuation.
- winterplace 5y ago
  Hi David. Do you have an email address or a method of contact?
mncolinlee 5y ago
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 overly worry that a colleague might get offended when you're simply trying to help. It's also not a chore anymore. Since the review is the job itself, it doesn't feel like a distraction. And since you're an outsider, you might know about best practices at your organization that the client hasn't been exposed to. When I review code, I read the summary explaining what that org likes in a review, but I also make sure to include tools and practices that they might not be aware of. In many cases, I can see they're lacking automated static analysis like ktlint/detekt and point it out. I might notice performance or security flaws that their own team wouldn't consider in a typical PR. While I actually enjoyed the style of work where reviewing a PR isn't a chore, there are a couple issues I'd like to see improved. Their rates could be improved for the best engineers. Also, the number of jobs isn't always enough for the number of reviewers. Gig work is much nicer if you can actually choose the hours and have more flexibility.
- andy_ppp 5y ago
  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?
  - mncolinlee 5y ago
    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?
  - andy_ppp 5y ago
nick007 5y ago
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.
- winterplace 5y ago
  Hi Nick. Is there a reason why PullRequest Reviewers from Canada are also included as an allowed country? Because that country has good rule of law?
FullyFunctional 5y ago
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").
- 6510 5y ago
  No one has to die or quit. You could just hire a new dev who doesn't have to wait for someone to explain the relevant parts of the godzilla object.
  - FullyFunctional 5y ago
    I guess we are in violent agreement then. I just happened to have fresh scars from a lot of knowledge that departed with an ex-colleague recently.
    - rmbyrro 5y ago
      I guess commenter would agree with you. They're adding a point that, on top of what you said, hiring a new dev (more likely and frequent in many cases than a sudden quit) would also trigger pains similar to a sudden quit.

andreyk 5y ago

While I agree in general, some forms of code reviewing require less context than others. They say "We review within your tools to catch security threats, stop crashes, and fix performance issues before they reach production.", and it does seem to me that these things are less a matter of style/context than just noticing potential issues. Not sure though, I had the exact same thoughts as you when first seeing this. Edit: to be clear, I personally don't see the value here, just playing devil's advocate.

jermaustin1 5y ago
But almost any good SA can catch all of those issues, a person doesn't need to look through the code for that.
- senko 5y ago
  This is very close to saying Halting problem is not, in fact, a problem: https://brilliant.org/wiki/halting-problem/ https://brilliant.org/wiki/halting-problem/
  - jermaustin1 5y ago
    Not at all, it is saying that if the only things this service is providing is a human doing SA (since they dont have any deep-knowledge of the codebase), then you could just use an SA. Code review in the real world misses the Halting problem, the only way you can really see it is if you know the codebase well enough to SEE it, and any 3rd party that is given a pull request to complete in a timely manner will not have time to fully learn your code base or even the modules you are submitting.
  - adgjlsfhk1 5y ago
    This isn't quite true because in most real world scenarios, one of the requirements for code to be correct is that it provably halts in a fairly limited amount of time.
  - reuben364 5y ago
    A human being doesn't solve the halting problem either.
  - samatman 5y ago
    Ironically, it very seldom is. It's Rice's Theorem which tends to get me in trouble!
HPsquared 5y ago
What will actually happen, is people will use these kinds of "context-free code review" services, then say a code review has been done. Technically correct in a narrow way, but not what most people would expect.
malux85 5y ago
I suspect that there’s only a tiny, tiny fraction of issues that are above the complexity that a linter and test framework can’t identify, but are below the complexity that requires deep codebase knowledge, and I would say that the overwhelming majority of value of code reviews is inside the issues that require deep codebase knowledge. Maybe this service could be good to point out superficial security bugs though, but the infrequency of these coupled with the effort of external human engagement I think would be a barrier. Also a few times in my career I’ve seen a team with no senior devs on it, they usually fail due to inexperience, maybe this service could help a team like this - who are producing a lot of really obvious mistakes …
alkonaut 5y ago
Code the falls between between "I need an experienced team member to see the issue" and "A few static analysis tools would no doubt have seen it" must be vanishingly small. For specific areas, I can see myself paying for external code review. For example some open source library authors sell it as a service, so they can review the specific parts of the code where the library is used. Rob Menschings FireGiant is one such example. Security reviews (Audits) I can also see a good use for. The current practice here is already to have external non-domain-experts review the code. So making that simpler or more frequent is a net win. The reviews I see little use for would be normal feature code, (pull request reviews) which need a lot of context and domain knowledge to even begin to review. Not audits, or reviews of specific areas or aspects of the code.

YPCrumble 5y ago

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.

blacklion 5y ago
It is very bold statement. Code models some real-world entities (domain). Code (completely with comments and documentation) cannot and should not document fully domain. It is context, which is needed to understand code. Yes, simple CRUD application can have all context encapsulated, but what's about some code which models, say, some aspect of chemistry? Should this code have enough context which includes several post-grad university courses? Or «simpler» example from my current $Job: we have a lot of code to build some models of derivative stock exchange instruments (options, futures, etc). Enough context for this code is, like, full shelf of 1000 page books. Good luck to review this code for everything but off-by-one errors if you don't work in this area for 5+ years.
- coryrc 5y ago
  Your example is, IMO, the exact use of this service. If you're a chemistry expert, you're probably not a coding one. These reviewers will ensure your code is testable and likely to do what you hope it does, in a way where your fellow experts can read and write their own automated proofs (tests).
alkonaut 5y ago
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 implemented using the right approach to begin with? Do we have other functionality that already does this? Does that other functionality use the same approach or is there good reason for this being different? Does this follow the business logic properly or are there any signs of misunderstanding the requirements? Are the requirements sensible?"

tyler_mann 5y ago

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 is degree that we help supplement the process and help catch many issues as well as inject a unique perspective. Our reviewers are all highly qualified, many are maintainers of popular open source projects or work at top tech companies. Our reviewers also gain context over time similar to a new senior engineer on your team. Reviewers also can share notes with each other to build up a corpus of information for your project over time.

Remed 5y ago
I wanted to join as a reviewer, but you don't accept reviewers from outside the US. Is this something what you are working on and has a chance to change in the foreseeable future? I work for a public US tech company but I'm located in Europe.
winterplace 5y ago
Hi Tyler. Is there a reason why PullRequest Reviewers from Canada are also included as an allowed country? Because that country has good rule of law?
gwbas1c 5y ago
For the past year, I've been working for a company where there are a lot of extreme novice mistakes in the codebase. Even though they reviewed their code, a novice developer reviewing another novice developer aren't going to catch things that are obvious to a developer with 5+ years experience. IMO: Target shops where they just don't have the expertise on-hand to do thorough code reviews. Don't waste time trying to convince a team full of experts with deep domain knowledge that they need you. (They probably don't.) We also have a "problem" where there are some components that are a different language than what most of us are experts in, so they end up being developed by a solo developer. When we need to jump in, as we learn the codebase, we also see novice mistakes that are very hard to fix, because we just don't have many years of experience in that language / platform. Thus, IMO, on your website, list out situations where shops will clearly identify a need for your service. (Team full of novices, solo developers, team members quit.) Don't go trying to convince "everyone" that they need you.

kevingibbon 5y ago

My company has used them before. I thought the same before using them. However I was surprised the level of talent they have. For a super senior dev the context that you need for most projects isn’t as much as you think. It was a net positive for us. Sped up our developer process given its so hard to hire senior devs right now. They also have domain experts. So say you are using some tech your team isn’t as familiar in. Great to get some extra eyes of that code to check for security issues, potential computation issues etc. Also they are much more broader than their company name suggests. Think of them as developers as a service. In this hiring environment it’s much needed.

walkingriver 5y ago
I'm one of those reviewers, and I agree with you about the talent. Not speaking about myself, but the people I've gotten to know and also the ones I have referred. Each PR gets two reviewers from pullrequest.com, and we get to see each others' comments. One will catch stuff the other misses, and we usually support each other. It's most fascinating when we disagree on something, which so far has always led to a high-quality discussion between the engineers and the reviewers. I've been working with them for most of 2021, and I can honestly say I'm impressed with the review comments I have seen. It's been nothing but respectful and professional. As a plus, it's made me a better code reviewer at my day job.
- winterplace 5y ago
  Hi. I have a question. Do you have an email address or a method of contact?
- gavinray 5y ago
  You do this on the side of your dayjob for extra income?
  - mncolinlee 5y ago
    I have certainly done this. PullRequest bought the Moonlight developer gig platform. A lot of full-time developers from Moonlight also took on gigs from PullRequest as they've been using the platform to sell their service. I'd guess most reviewers have other jobs or contracts.

koblas 5y ago

As somebody who's done a bunch of reviews on the platform you realize a bunch of things about development that we don't want to admit. When I review code for my own team (for my day job), there are many times where internal pressures on my time will make me prematurely stamp LGTM on one of my co-workers PRs that I trust. When I'm doing reviews on PullRequest I remove the "trust" marker in my review along with this strange thing called "economic compensation" where I'm paid to spend time working on it, rather than having somebody ding me for not completing some other task that needs to ship this week so I spend more time reviewing the details. I think there are a few levels of code reviews that should be in place. * Automated checks eslint/typescript as examples, you would be surprised at how many companies don't have this! * Best practices -- react hooks, golang interfaces, calling conventions... * Testing -- are the tests structure to test good and bad, are they brittle? * Security -- Did you build the right IAM role in terraform, did somebody just checkin their GITHUB key (ok, that should be automated). * Performance -- Is that Promise.all going to dispatch 1000 calls in parallel. * Architecture and inter dependancies, there is a limit for a 3rd party here. Now if you're the Lead/Architect on a project and at a minimum you can outsource the Best practices / Testing portion of the pull request to a 3rd party you now can focus on the architectural dependancies that are really what you care about. As an architect you can easily provide reviewer notes to the person doing the review that you are interested in focusing on specific areas of improvement across your team. Giving you the coverage to focus on the high level issues and inter dependancies while not focusing on variable names or test coverage. Your time is valuable, you should spend it on character development not on the punctuation.

winterplace 5y ago
Hi David. Do you have an email address or a method of contact?

mncolinlee 5y ago

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 overly worry that a colleague might get offended when you're simply trying to help. It's also not a chore anymore. Since the review is the job itself, it doesn't feel like a distraction. And since you're an outsider, you might know about best practices at your organization that the client hasn't been exposed to. When I review code, I read the summary explaining what that org likes in a review, but I also make sure to include tools and practices that they might not be aware of. In many cases, I can see they're lacking automated static analysis like ktlint/detekt and point it out. I might notice performance or security flaws that their own team wouldn't consider in a typical PR. While I actually enjoyed the style of work where reviewing a PR isn't a chore, there are a couple issues I'd like to see improved. Their rates could be improved for the best engineers. Also, the number of jobs isn't always enough for the number of reviewers. Gig work is much nicer if you can actually choose the hours and have more flexibility.

andy_ppp 5y ago
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?
- mncolinlee 5y ago
  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?
- andy_ppp 5y ago

nick007 5y ago

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.

winterplace 5y ago
Hi Nick. Is there a reason why PullRequest Reviewers from Canada are also included as an allowed country? Because that country has good rule of law?

FullyFunctional 5y ago

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").

6510 5y ago
No one has to die or quit. You could just hire a new dev who doesn't have to wait for someone to explain the relevant parts of the godzilla object.
- FullyFunctional 5y ago
  I guess we are in violent agreement then. I just happened to have fresh scars from a lot of knowledge that departed with an ex-colleague recently.
  - rmbyrro 5y ago
    I guess commenter would agree with you. They're adding a point that, on top of what you said, hiring a new dev (more likely and frequent in many cases than a sudden quit) would also trigger pains similar to a sudden quit.