19 ms·

Code Review as a Service

• smalter 5y ago
  i've used pullrequest a bunch and had great experiences with the product and company. mostly used it in small companies with 1-2 devs where it helps to get another pair of eyes on it. also, helpful as career development / learning for a junior dev. happy to answer any questions.
• antonpuz 5y ago
  I've greatly improved as a developer by reviewing code, this improved my project and improved team alignment, that's much more valuable than getting inputs from human linter.
• dennisy 5y ago
  Interested to know if anyone has used this service or similar and what the quality is like.
  • codingdave 5y ago
    It is new to me, but raises a concern that they could only review for general engineering quality, not whether or not a PR is appropriate to a codebase. How would they know if the same problem has already been solved elsewhere, that no wheels are being re-invented, or that a PR is stepping on the toes of something else the team is working on. Because that is the value I've found in code reviews - not generic "is this code elegant?", but "does this code play well with what everyone else is doing?"
    • Leherenn 5y ago
      I guess you could first have an internal architect or similar vet the PR before handing the PR to this service for the "technical details". As you said, the big value in reviews are the points you mentioned. Correctness/technical quality certainly has value, but at $700/dev/month the reviews better be really good. Especially since doing it internally has value as well (knowledge sharing in particular).
• walterbell 5y ago
  > expert engineers backed by best-in-class automation. "Automation" = static analysis or something like https://codeql.github.com/ https://codeql.github.com/?
• aneutron 5y ago
  I will admit that I thought this was a joke on the "SaaS" everything trend. I can see how this works for fairly limited web applications for example, but as soon as the application grows in complexity and interacts within a bigger system of systems, I am doubtful that it would be logistically possible to outsource the code review (legally, knowledge transfer wise, and a plethora of other angles I'm a tad bit lazy to consider). Overall, why not, if it's priced correctly, then it's probably a set of additional eyes for small projects. But for anything medium or higher, yeah, I don't see this realistically working. Maybe OP (?) can explain if I'm wrong (very likely). There's probably something I'm missing.
  • dennisy 5y ago
    My thought is the same - I was wondering if I had missed something! I really need to find a good mentor for our project :(
  • whazor 5y ago
    Overall, for bigger and more complicated systems you would have an architecture that is peer reviewed and communicated internally. Afterwards you have individual components that have their code reviews and still need to follow industry best practices. I think external code reviews is great idea as it could allow the team to focus more on conceptual reviews and consequences to other systems.
    • noduerme 5y ago
      I've yet to see a company ask for feedback once they were done pushing out a half baked API. And if anyone uses it, they do get plenty of feedback. As far as whether the code is internally badly written... if the guy writing the code can understand the suggestion, he's probably already done it or decided why not to do it. If he can't understand the suggestion, then it's a waste of money.
• niros_valtos 5y ago
  Saw this link here in the past. Didn’t pickup. The reason why I don’t like it is that random people, regardless their expertise, cannot just review PRs and understand the impact of the change on the system without being deeply involved in the product.
  • jopsen 5y ago
    Nor can arbitrary people weigh the balance between accepting technical debt and shipping a feature. It involves a risk / benefit analysis. On the other hand, maybe you can build a relationship with contractors that focus on reviewing code. And maybe it can give insights your team won't have?
  • rwmj 5y ago
    I'm a little more positive about this. A careful code reviewer might be able to spot generic security or even logic flaws in code, such as inappropriate use of strcpy in a C program or code which is unreachable in a way that cannot be detected by the compiler. Also there's a lot of scope for automation, such as running Coverity or other free and commercial linters/checkers, although also a danger of overwhelming the results with false positives and junk.
    • niros_valtos 5y ago
      Results from these tools typically go to the developers, not to the PR reviewers. They see only the result of fixing a vulnerability, and if there is a new vulnerability introduced, these static code analysis tools should detected them before deployment to dev/prod.
• hankchinaski 5y ago
  Having on-demand engineers look at code, without broader context on the project it is in my view the same as something that can be automated either or both via static analysis and custom ci/cd workflow checks. This can probably makes sense on a project with more junior engineers where many basic improvements can be supposedly suggested without needing broader context? I would be keen on hearing the use case
  • adamnemecek 5y ago
    I have reviewed for pullrequest and you’d be surprised how many things one can fix. Why do you think that you don’t have context? You can look at the whole project and see what’s up. And no, you can’t write programs to do this.
    • noduerme 5y ago
      It's a rare case indeed where I'd be willing to let a third party in on business logic plus the whole source code for something I was developing. I love talking shop but if I'm really having a problem with a structural issue, it's hard to imagine someone outside giving any better advice than people inside who understand the lay of the land. Meanwhile, it's a pretty much totally unacceptable security risk. So what's the upshot to this over an internal code review?
      • alexdowad 5y ago
        Well, maybe the people you have available to review code aren't all that good. Nobody ever wants to admit that their team is below average, but 50% of teams are. Maybe due to limitations on team size, it's not possible to have an in-house expert on every technology used.
    • tialaramex 5y ago
      Variable naming is a really obvious example of "you can't write programs" to assess this. Because it needs some general intelligence. Are very short names OK? Generally not, but x is a perfectly good name for an x coordinate in a graphing application for example. On the other hand methods named colour (to get the shade) and shade (to get the colour) need some serious documentary explanation of what the hell you're up to even though in themselves they're acceptable names. Language idioms, and (if this service is expensive enough) per-project idioms are not usually or reliably machine checkable. Also beginners make lots of confusing mistakes, a program may end up missing the woods (e.g. this should just be an iterator, 90% of the code is mechanics that are doing what the language's built-in iterators do) for the trees (the variable names used for all the counters being tracked are bad)
• grenran 5y ago
  There's no way that Google would actually use this service.
  • ceejayoz 5y ago
    SaaS apps tend to slap a logo up when there’s a single free trial account with a @google.com email address. (In this case, they seem to be claiming Google engineers are moonlighting as reviewers, not using the service for reviewing Google’s code.)
    • pledg 5y ago
      They have the Google logo in both "Work with world-class engineers" and "See why thousands of teams trust PullRequest". With the latter being next to the customer testimony section, if they are only suggesting that Google engineers do the reviewing that's quite disingenuous.
    • adamnemecek 5y ago
      Google has invested in pullrequest and some engineers review for pull request https://techcrunch.com/2017/12/07/pullrequest-pulls-in-2-3m-seed-round-led-by-googles-gradient-ventures/ https://techcrunch.com/2017/12/07/pullrequest-pulls-in-2-3m-...
• mytailorisrich 5y ago
  This implies that you have no-one in your team to do code reviews and that you're fine allowing random third parties access to your infrastructure and a peek into your projects and code base... Both the premise and the proposed solution sound very odd to me.
  • walkingriver 5y ago
    Most projects I have reviewed for pullrequest.com have reviewers on the engineering team also. In a way, we are helping them get better at reviewing their own code. I imagine that some teams won't need us after a while. On the other hand, in some projects we have become trusted members of their teams.
• cptaj 5y ago
  I've never seen outside consultants make useful contributions to a project. This is even worse. Thanks, I hate it.
• vemv 5y ago
  I'd have some amount of healthy skepticism over obvious concerns (most prominently, keeping IP safe) but it also it seems worth a shot; it solves a real problem because often for teams code reviews are a bottleneck and bit of a battlefield.
• pelasaco 5y ago
  I'm imagining that to make such service profitable, offering something like $699 per developer per month, you are not hiring reviewers from USA, right?
  • Recursing 5y ago
    From the FAQ: > Reviewers earn anywhere between $50 and over $3,000/week. Earnings are based largely on the amount of time spent reviewing on the platform and the type of code being reviewed. PullRequest’s payment rates are comparable to those of a senior-level engineer based in the US. > PullRequest issues weekly payments based on review activity during the preceding week. The time that you spend reviewing is tracked through our platform; reviewers are not required to log hours or invoice.
    • pelasaco 5y ago
      > Reviewers earn anywhere between $50 and over $3,000/week. They are operating just like a freelancing platform, right? How many hours is a "week" for them? 40 hours? > PullRequest’s payment rates are comparable to those of a senior-level engineer based in the US. Like include life and health insurance, paid vacation, profit-sharing, a generous signing bonus, and more?
      • lambic 5y ago
        Why don't you say what you're thinking instead of disguising your opinions as questions?
        • yjftsjthsd-h 5y ago
          Let me try: This looks way too cheap to be able to afford good enough reviewers to be worth using; if someone is good enough to be able to pick up a new codebase and usefully review changes that quickly, they'd get a better paying job.
          • breakfastduck 5y ago
• cannonpalms 5y ago
  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.
  • SomeHacker44 5y ago
    Maybe for you. For others it can have all or any of those other things as its primary purpose. One important thing for me is that it prevents or at least mitigates unilateral insider attacks by having two people required to change code.
    • astrobe_ 5y ago
      Having a bus factor>1 is still a major prerequisite for doing that. Because if you have a junior developer review the changes of a senior developer, there are so many social engineering tricks to make the backdoor pass the review that it isn't even funny.
  • pelasaco 5y ago
    "but they have AI".. probably not. Probably the same linters and scanners that we all know + cheap working force reviewing code manually.
  • mfashby 5y ago
    Hmm, what about education? I've learned a bunch through thoughtful reviews of my code by other devs.
    • loloquwowndueo 5y ago
      That’s them transferring their knowledge to you :)
  • beanjuiceII 5y ago
    i thought code review was to assert dominance and to block as long as possible for yak shaving reasons; But I do like your version of it! Happy holidays~
• softwaredoug 5y ago
  On the one hand, lacking broader context might make this service seem silly. However I think it can help bust groupthink It’s still valuable to get more general feedback and ideas explicitly without context to challenge our attachment to current practices in existing codebase schools. It’s nice to get a pair of eyes with a completely different background give feedback. With context we may have blinders on. A fresh perspective might uncover things we haven’t thought of and bust groupthink Even though I’ve been coding in Python for years, I still might not have awareness of the best most effective way to do something in general. Imagine all the projects started in the last few years from people learning Rust for the first time? It may not make sense for the largest, most complex code bases, but I can see it valuable for medium to small projects to get outside perspective.
• loloquwowndueo 5y ago
  “LGTM” as a service :)
• privacyonsec 5y ago
  Isn't paper peer reviewing broken ? Want to do the same thing with software?
• jph 5y ago
  Good code reviews are superb for helping teams accelerate into new technology areas, frameworks, languages, and integrations. As a top of mind example, when a team wants to spike on migrating from C++ to Go or Rust, including using libraries and porting services, then I see very high value in paying for skilled contractors to do code reviews-- because what your team is gaining on-demand upskilling.
• lambic 5y ago
  The only time I can see this being useful is on solo developer projects, and solo developers probably can't afford it.
• alkonaut 5y ago
  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.
• booleandilemma 5y ago
  And I’m guessing their reviewers work for free, like on Code Review Stack Exchange? Or are these people working for relative poverty wages overseas?
  • senko 5y ago
    > And I’m guessing their reviewers work for free, like on Code Review Stack Exchange? No need to guess, it takes 20 seconds to check: https://app.pullrequest.com/signups/reviewer https://app.pullrequest.com/signups/reviewer
    • GrahamCampbell 5y ago
      Reviewers are well-compensated, meaning pullrequest.com can attract very high quality reviewers.
  • walkingriver 5y ago
    I am a reviewer on pullrequest.com and I can assure you I do not work for free. Many of us are Senior or Lead developers with day jobs, who do this as a side gig and to keep our skills sharp. During my code reviews, which tend to revolve around Angular and Ionic (that's what I signed up for), I have found lots of outdated practices. I can often provide advice on how to make their code better, show them features that are deprecated and how to address them, and generally make their code better. As another reviewer has pointed out, we can see the entire code base, not just the current diff. We tend to work with the same companies repeatedly and become familiar their project. Some of the engineering teams treat us as part of their team and ask for advice, which is really cool. Doing code reviews for pullrequest.com has also made me a better reviewer in my day job and has changed the way I approach my coworkers. It's truly been a win-win.
• srhaegrfes 5y ago
  Putting aside IP and feasibility concerns, what is the longterm roadmap? This is obviously not a VC scalable product business if it acts as a broker to (albeit expensive) human consultants. Are they hoping to get enough training data from the consulting practice to bootstrap an AI code review product?
• gandutraveler 5y ago
  On top of what others have already flagged this is a big no for any companies security & compliance. Why would any company share their private codebase ?
  • ddm379 5y ago
    Disclaimer: I'm Dan Mateer, COO at PullRequest Great question; security and compliance is a very big consideration for our customers. All code review on PullRequest is done within the platform, engineers in the PullRequest network cannot clone branches like in a garden variety source control, and we have a number of tools to give clients as much control as possible as to what our platform and engineers in our network are exposed to (e.g., https://docs.pullrequest.com/pullrequest-docs/code-review-settings/restrict-files-and-directories-from-pullrequest-review https://docs.pullrequest.com/pullrequest-docs/code-review-se...). We work very closely with our customers to ensure configurations are set up to provide our engineers with adequate context while limiting or outright restricting exposure of things they want private private. This is also a big part of why PullRequest Reviewers are by and large restricted to US-based engineers. This ensures accuracy and consistency of criminal background checks and ease of enforceability for our non-disclosure agreements. From a legal risk assessment perspective, using PullRequest is similar to hiring a technical consultant.
    • winterplace 5y ago
      Hi Dan. Is there a reason why PullRequest Reviewers from Canada are also included as an allowed country? Because that country has good rule of law?
• imglorp 5y ago
  I hope my boss doesn't think, "It's cheaper to pay for this service than for us to review our own PRs. Outsource ++!"
  • GrahamCampbell 5y ago
    Cheaper in the long term to do both. :)
• swalsh 5y ago
  I'm not sure this is a good idea for enterprise startups building closed-sourced software. As a software architect, some things you just don't delegate. Code Reiews are some of the most important things I do. But (and please don't downvote me for this, I know there's an instant reaction to downvote anything blockchain related) for DAO's this would be amazing. A DAO (Decentralized Autonomous Organization) might be running a software as a service. But it might not have any full time employees. It might not have any employees at all. A lot of updates to the software might come from random people (or bots). The DAO will need to evaluate and pay for any of those updates before it decides to merge the pull request. A code review as a service would be an absolutely invaluable tool for a DAO with a software product that doesn't have any full time architects to perform the service.
  • scubbo 5y ago
    I didn't downvote you, but I'd love to hear some situations in which this hypothetical DAO would be comfortable running code that it (where "it" means "an engineer who has a stake in the DAO and so is invested in its success") had not reviewed? How would it trust the results of this review? I'm trying to remain open-minded and curious about blockchain-related use-cases.
• throwaway984393 5y ago
  I really love this idea. It's like static code analysis but with more specific feedback based on context. This could dramatically improve code written by junior and middle level engineers. I'd use it! The only stumbling block seems to be that a lot of devs seem to be resistant to it. I'm not sure why that is; there's many forms of code review and feedback, from shallow to very deep. And I've seen many teams that fail to do proper code review. This could be an excellent introduction to proper practice for immature teams.
• ralusek 5y ago
  A service like this would only be able to find a certain class of issue. Basically syntax, but not semantics. There is no substitute for deep knowledge of a particular codebase.
  • walkingriver 5y ago
    A really cool side effect of working with pullrequest.com over the past year is that I do feel that I've gotten to know some of these projects. Some interactions end up becoming lengthy conversations between the reviewers and the engineering teams over the course of multiple pull requests. We point out potential issues and are still around when those issues are addressed. For some projects, the reviewers are treated like part of the team.
• rpunkfu 5y ago
  I wish person (people) involved in making this would do some more research, it's completely missing a point of PR review. This is to me a set of linters with human error involved..
• nkmnz 5y ago
  I'm currently the only developer in my startup and I crave for high quality feedback, so this service sounds like it's a gift sent from heaven – especially for the $699 price tag per month. I guess they probably make a mixed calculation: take a small company with 10 devs. Two of them are part-time, on any given week one of them is on holidays/sick leave, one is very junior and doesn't contribute much yet, another one is more concerned with project management than with writing code and one has in fact transitioned into management but remains on the dev team because it's part of their identity to still "get their hands dirty" (even though they don't have the time to produce much code) – so you might end up with sth like 4-7x more code than I write, but 10x the payments. My productivity is not above average, but I have very little overhead besides coding. Unfortunately, I cannot find any information about a minimum team size for the PRO plan. I think I'll give them a try.
• napolux 5y ago
  LOL. This will become in less than 1 month a "LGTM" feast. 3rd world countries developers (organizing themselves in groups to pass the intro test from the website) giving green lights to random people around the world for peanuts. This is a clever business model if you ask me. Let's be clear for a second. Nobody, not event the legendary 10x developers can review properly code without context or everything mentioned here: https://news.ycombinator.com/item?id=29624787 https://news.ycombinator.com/item?id=29624787 If we want our code to be approved by random people doing it only for money, this will work for sure, but it's not how it works in the real world.
  • ivanche 5y ago
    It seems they don't accept any reviewer outside US and Canada, let alone 3rd world countries. https://www.pullrequest.com/faq/ https://www.pullrequest.com/faq/
    • napolux 5y ago
      Thanks, but that's not enough :)
      • ivanche 5y ago
        As an outsider, I can't say anything more. I know that US/Canadian developers are in no way guarantee that LGTM feast won't happen but that's PullRequest's policy.
  • dhimes 5y ago
    Could you clarify what you mean by "LGTM?" Just people saying, "looks good to me" and doing crappy reviews, or is there some LGTM "thing" out there that everybody will use? Or something else?
    • MajorBee 5y ago
      There is a meme out there in the wild (or at least, I think there is a meme) that many reviewers would simply approve PRs blindly leaving a LGTM (looks good to me) comment when their internal monologue is closer to "...I have no idea what's going on here and I'm too afraid to ask at this point". I suppose it could also be sheer laziness of not wanting to parse many lines of new code, especially if you're in a situation where you don't have a 100% tight understanding of the codebase (as I have been in many a times).
• est 5y ago
  I hope it can review popular code on github.com automatically.
  • deleted 5y ago
    [deleted]
• lmilcin 5y ago
  Except I don't believe in code reviews. Over the years I have experimented with pair programming and I think it is way better solution. The one catch with pair programming is that some people just prefer to work alone and it is really draining to be talking constantly for couple of hours. I have modified the system to do on/off pair programming (ie. split up a little bit of work, rejoin later in the day, share what we have done, and work a little bit together). The basic, unsolvable issue with code reviews is that the review is done after the code has already been written. As you know, the cost of fixing a problem is larger the later in the process you catch it. Pair programming aims to accomplish the correction while the code is being written. Another huge problem is that, because the reviewer is not taking part in the development, he/she does not have the same level of understanding of what was supposed to be done. Also, code reviewers are typically disincentivized from doing review well: * They have other tasks to accomplish, review takes their time away from those tasks but the deadlines are not pushed automatically, * The review tends to land at a random point in time disrupting their flow -- they have something else in mind already and they want to switch to their work as quickly as possible -- meaning they will not want to get into great detail with understanding the problem. This causes reviews to usually be very shallow and focused on trivia. Usually, I see reviewers read the code file by file line by line, hundred times faster than it was written. It is absolutely impossible to verify a large change like that and this guarantees that they will not actually verify it thoroughly. Yeah, you may find superficial flaws, but that's about it. Other problems: * The developer feels resentment because he/she thought it was all done. * A lot of effort was spent on a wrong solution which is lost productivity. * From project management PoV it is a problem because we can't tell how much time/effort it will take until last second. * Reviewers feel pressure to find something so they will just report trivia if they can't find real issues. * Reviewers tend to not want to report huge issues that would require complete rewrite because they are developers themselves and wouldn't want the same happen to them, and also because they are typically members of the same team. * and so on.
  • ivanhoe 5y ago
    I agree with your points, but the pair programming also has downsides, main one being that it doubles the total cost per task, and also it's usually limited to 2 pairs of eyes (again because of the cost), while code reviews can (and should) be done by as many members of the team as possible.
• GrahamCampbell 5y ago
  I have performed reviews on pullrequest.com, and lack of context tends not to be an issue. Code review is an interactive process, where questions can be posed to the PR authors, and PR reviewers have access to search the codebase. Customer success at pullrequest.com also provide context for the repo and organization working practices, attached at the top of PR descriptions, to help reviewers with context and to know what kinds of feedback are useful. Sast and Dast are still not sophisticated enough or widely used enough to be good enough. In-house senior and principal engineers often do not have the time to be on top of every repo and every PR in their organization, and need to outsource an extra pair of eyes. It is highly common that I am reviewing codebases that do not have any Sast or Dast, or have workarounds to silence the tools, resulting in bugs and security issues slipping through. It is very rare that I am able to approve PRs without comments, even after an internal review has taken place!
  • robocat 5y ago
    > Sast and Dast SAST, or Static Application Security Testing DAST, or Dynamic Application Security Testing https://www.softwaresecured.com/what-do-sast-dast-iast-and-rasp-mean-to-developers/ https://www.softwaresecured.com/what-do-sast-dast-iast-and-r...
  • winterplace 5y ago
    Hi Graham. Do you have an email address or a method of contact?
  • spmurrayzzz 5y ago
    Just curious about the flavor of code you were reviewing. Can you elaborate a bit on that? Specifically - any software integrations with hardware? E.g. maybe a distributed cloud service that talks to on-prem firmware or similar (think IoT, cisco/juniper network gear, etc). There is a whole class of software that I can't imagine someone could come in blind to and offer any real value to necessitate the balance sheet exposure. But I'm very interested to hear if folks there have had to do this (yourself or others).
  • dm03514 5y ago
    > Code review is an interactive process, where questions can be posed to the PR authors, and PR reviewers have access to search the codebase. I also performed reviews on pullrequest.com (~2+ years ago?). I firmly believe code reviews are an interactive and knowledge sharing process but was explicitly told NOT to ask questions during pull requests, but instead to make statements focusing on bugs and style. Based on the comments here, this def looks valuable to some of pullrequests.com customers. I believe this is more of a human acting as a safety net, or risk mitigation strategy, and loses the majority of the value that pull requests can provide to an organization.
• dirtbag__dad 5y ago
  I used this service at my previous employer to launch a small django/nextjs app and it was mostly fantastic. Background is that I worked at an VC-backed startup as a dev after doing General Assembly’s full stack bootcamp. Left that job to do ops/growth, and ~2.5 years later volunteered to build the web app when my company put the project on their roadmap. As the only developer at the company, pullrequest was great for: - a general gut check on how I was doing - recommendations on how to better write js/python. linters help but nice to have a person offer feedback on more advanced ways of doing things - sourcing documentation on best practices. I found a lot of typescript/JavaScript resources to be inconsistent/confusing. Was great for someone to find and vet guides for me. - help with bugs/errors - basic library choices and architecture decisions It was also fantastic to have several people reviewing my code at once. Gave me a perspective on the type of engineering manager I’d want to work for. Some folks focused more on technical details but struggled explain their changes in plain English, while others seemed to be the other way around. pullrequest was not great for: - doing things fast. They didn’t have a real-time messaging feature so I’d get hung up on waiting for feedback. They do have a 24(?) hour turnaround, but when several folks are commenting on the same PR it gets hard to track what changes _really_ matter vs what they are throwing out there as a nice to have. - Anything that required context outside of the files committed. Though with some extra long PR comments I could manage. I would 100% recommend pullrequest for small teams who are heavy on more junior devs or migrating to a new stack. It is an inexpensive way to ramp up learning. Last thing here — when I worked at the startup my code was rarely reviewed, and I’d have to actively ask for it. Not all companies follow best practices (even if they have the resources to). I would’ve loved this at my past job, too.
• gorgoiler 5y ago
  The best code reviews are ones that leverage experience from the reviewer to stop logic errors. Your code may well compile and the tests you concocted may pass but that doesn’t mean your code should ship. Without a second pair of experienced eyes looking at your crap, how are you to know that: - (general experience with modelling) what you felt should be a single class would be better if it were factored into two classes; and - (specific experience with this particular codebase) the second of those classes already exists in the codebase and can be found in com.clown.util? The former can be helpful but it’s just noise compared to the value of the latter. How does this service avoid being just a human powered style linter?
• 8organicbits 5y ago
  I noticed this requires LinkedIn for signup, that's too bad. I deleted my account years ago once it became a major source of spam. I think this is the first place I'm seeing it required. Strikes me as an odd requirement.
• whoomp12342 5y ago
  very often code review requires domain knowledge. This seems like it would devolve into arguing over coding preferences
• nyanpasu64 5y ago
  I noticed the example image: "It looks like this method needs to use the read lock because it's accessing the `dataKeys` map. I would recommend documenting which methods are and are not meant to be safe for concurrent access." To me, this is a problem addressed by Rust's &/&mut separation, which can be imitated in other languages (eg. in multithreaded code, wrap types in Mutex<T>/MutexGuard<T> or RwLock<T> with read guards lending out `const T&` and write guards lending out `T&`, if you're in a language without const, tough luck). This creates code which self-documents concurrency in the type system like Rust, though it's less watertight since you can hold onto references for too long.
• schnika 5y ago
  I wanted to sign up as a reviewer but it seems like one needs a linkedin account to do so. Since I don't have a linkedin account it seems like there is no other chance?
  • Cakez0r 5y ago
    Me too. Hopefully someone working for the company sees this and has some sway to remove the requirement.
• natded 5y ago
  Teaching sand to think was a mistake.
• gitdiff 5y ago
  As a PullRequest reviewer, I think many on this thread are missing the forest for the trees. First, I think many here may be viewing Code Review as a Service from their frame of reference: from unicorn start ups, FAANG tech companies, prestigious universities, etc. Many of the companies that we help aren't coming from this world. They're small startups, looking for technical guidance. They're entrepreneurs who need to ensure they're not being duped by app developers. They're older, less tech-savvy companies that are looking to modernize. These companies need help, and they can get help from people who have technical expertise. (And by the way, some companies aren't sophisticated enough to set up linters or code scanners at their stage of development) In a similar vein, many of the developers we help may not have had the same level of education, learning, or coaching as you or I may have. For example, I've helped introduce more modern syntax options to developers, such as string interpolation and extension methods in C#, to Options and Streams in Java, to filter/map/reduce functional patterns and optional chaining in Javascript. These developers may have never seen high quality code or had mentors who insisted on a high bar for code quality. Even for more sophisticated customers, I've left comments ranging from security vulnerabilities (e.g. SQL injection), errors in boolean logic, recommendations for improved test coverage, recommendations for simplifying code (e.g. creating reusable functions), preventing race conditions, and more. I've also reviewed candidate assessments to help unburden senior engineers so they can focus on writing code. Sometimes, the proof is in the pudding. There's a market for these services and that's why some companies pay for them and why I get to review code on demand. I periodically get feedback from the teams I help that I've done 'Nice Work', receiving positive ratings from the developers I review for. I'm proud that I can lend my expertise to make code better.
  • tanjiro 5y ago
    100% agreed. This is probably useful to enforce standards in a team that might be lacking it otherwise. For big tech teams, it might be useful to do such CRs randomly to make sure devs in the company are aligned on best practices.
• deeveloper 5y ago
  I'm a reviewer and a firm believer in PullRequest. I do believe that some of the concerns expressed here are valid but I think they belie a fundamental misunderstanding of "what you're getting" from PR. Having an under-experienced contractor review your code without context, pointing out things that static analysis could/would catch sounds like a terrible proposition, but that is NOT what PR provides. Speaking from personal experience, the reviewers are very knowledgable professionals who are not doing this for money as the primary motivation. The pay is great but is more of a perk/reward for doing something we enjoy, and doing a great job at it. The staff at PR does an amazing job of coaching and guiding reviewers on how to provide the most value to the client and also encouraging us to do our best work. It is never anything like "You need to be faster and bill less, do more, etc" it is quite the opposite, we are encouraged to "keep the clock going" while we research, gain context, etc. Everything is focused on providing value to the client, and IMHO a stellar job is done. Every review comes with detailed information on who the client is, the make up of their team (seniority, etc), recommendations on what to look for (and what NOT to look for) etc. Essentially you are putting your code in front of people who have been there, done that for a long time and are acutely aware of not only risk but also pain points and how to avoid tech debt. We are providing insight, recommendations and even code snippets on how to avoid repetition, speed things up, or make them easier to maintain.
  • winterplace 5y ago
    Hi. I have a question for you. Do you have an email address or a method of contact?
• adamqureshi 5y ago
  Im a 1 MAN SHOP and i hire contractors ( engineers). I am not sure if this a good fit for me? My main concern is of course cost. Do you provide feedback on what todo with the software stack? ( can my guys implement a new plan provided by you?)and 2nd question is do you provide pay for service, for example if i am interested in a new software architecture and my guys just code it up / follow your plan / recommendation. Thx
• andrewmwatson 5y ago
  I've been a reviewer on PR for a couple of months now and it's been amazing. I've gotten to help dozens, maybe hundreds of developers improved their code, fix security, readability, testing and lots of other issues. Sometimes the PR is short and the review is short but sometimes it's much more involved and I've had the opportunity to really explain things and educate people in a way that no lint checker could. I'm able to apply the wealth of my experience and knowledge with specific guidance based on things I've learned from building software for 22 years. I can't think of another way that a company could gain access to that kind of guidance for only $699 a month. I can go into detail explaining how to safely and reliably structure something and the feedback I get from the developers is positive and appreciative. I also collaborate with other reviewers about things we're seeing and I've learned a ton in the process because the other reviewers they've all got such a depth of experience and knowledge in different areas. I don't do reviews full time, I have a day job, but I've been earning about $1000 a week doing it just some nights and weekends and that has been really great too. Overall, I think PullRequest is an enormously positive thing for its customers and the reviewers. We're not trying to replace people's existing review processes if they have them. Our goal is just to add to our customers' capabilities and I think we're going very well at that.
  • janikvonrotz 5y ago
    Marked as spam.
• Kocrachon 5y ago
  I understand NDAs are a thing, but I still don't think I'm too comfortable with the idea of letting a bunch of third-party people look at a bunch of my internal information. And I understand this is targeting startups a lot more than large established companies. But to me that's even more concerning because as a startup you're really trying to move fast and hoping someone doesn't beat you to the punch, and you're handing a bunch of people you don't know how much of your secret internal information, and hope that they don't go talking to other people about it.
  • vincentmarle 5y ago
    Most startups are too busy finding product market fit than trying to worry about sharing 'secret information' (your actual secrets shouldn't be in your repo anyways).
  • robocat 5y ago
    I think most engineers (and execs) overvalue secrecy. My limited experience is that access to your code is really not that valuable to competitors. Code might be valuable to hackers wishing to targeting you, presuming your business is a valuable target.
• seneca 5y ago
  This post has a number of what look like astroturf advertising comments.
  • darkstar999 5y ago
    That's the word I was looking for. It really does look like that.
  • robocat 5y ago
    “Please don't post insinuations about astroturfing, shilling, brigading, foreign agents and the like. It degrades discussion and is usually mistaken. If you're worried about abuse, email hn@ycombinator.com and we'll look at the data.” - see https://news.ycombinator.com/newsguidelines.html https://news.ycombinator.com/newsguidelines.html
    • seneca 5y ago
      Yes, you're right. I found this example pretty blatant and egregious, and my annoyance with it got the better of me. I should have handled this differently, thank you for the reminder.
• dawnerd 5y ago
  I had to flag this after seeing several obvious spam replies trying to boost the service. Come on now.
  • darkstar999 5y ago
    The many multi-paragraph praises from green accounts did it for me.
• skeeter2020 5y ago
  This reminds of the first (and only) time I had a full 360 degree review: contradictory, zero-context opinions from people I don't know very well who haven't earned the right to interact with me in a way required for this minefield of feedback.
• akhil-ghatiki 5y ago
  How does this play with the Non disclosure agreements ?
  • akhil-ghatiki 5y ago
    ignore this comment. They have a background check and NDA process.
• soheil 5y ago
  I lot of people are saying CR is a way to transfer knowledge within a team. I think they're missing the point. Sure, you can transfer knowledge within a PR but don't you think that's a bit late? By the time CR is requested, the code has been written, the architecture has been thought of and so much time has been wasted doing the "wrong" thing. It's a horrible way to learn/train.
• nathanielarmer 5y ago
  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 PullRequest service won't catch every bug. As reviewers on this service, we lack context* to fully understand the impact of every code change. However, this can be a blessing in disguise. As an outsider, I bring an entirely different set of context to the project. I can see errors or improvements that teams have become blind to, I don't have the pressure of shipping for X release, and I've often been not only where these teams are, but where these teams want to be in X months. This is all done without using any man-hours on the client team - which is often a critically short resource. Ultimately the proof is in the pudding, I raise comments on almost every single review I do, raising from best practice to architectural to security vulnerability, and the majority of the time teams take that feedback onboard. Other QA: Where are you located? I'm located in San Francisco. Who approves PR's? I tend to consider my role to advise and sometimes mentor. I will give opinions, but it ultimately the client's job to approve/reject PR's. My code is great, I don’t need this! Have you tried it? As an outsider, I catch peoples blind-spots. That said, PR is always looking for great reviewers to join the team! * It's worth noting, that between seeing the code, optionally having access to the full code base, and asking questions of developers - I develop a decent mental model of most projects.
  • winterplace 5y ago
    Hi Nathanie. Do you have an email address or a method of contact?
  • Jach 5y ago
    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".
• joshgrib 5y ago
  Can someone give a solid and succinct review of doing this as a side-job? What was 1) the hourly pay, 2) language(s) you reviewed, and 3) how was the work?
  • nathasm 5y ago
    Sure, I'd be happy to give you some insight and background. A little background: I've been doing software dev for 20+ years ranging from C++, Ruby (not Rails), front-end/full-stack dev. I make enough in my day job to be plenty happy. I won't comment on pay (it's good enough for me!) as I'm not sure if it's standardized across the board, but let me just say that I was going to start moonlighting since my current role isn't as much development as I am used to....but I decided that PR was going to be a better fit and here's why: 1) I enjoy mentoring! I love interviewing candidates, doing reviews for my teammates, being a good role-model for junior devs to follow. I've taught and helped folks transition from non-tech to a tech jobs. And as a reviewer for PR, it's a lot of the same sort of thing that I already enjoy doing! 2) It helps me become a better programmer! Seeing novel or interesting solutions is always fascinating to me. I love to learn something new, and reviewing code I'm able to see the mistakes (or learn what didn't work) and then able to then take that knowledge and pass it on! 3) Much more flexibility. I can work as little, or as much as I want. I don't have a client asking where the MVP is. I don't have to worry about project planning, hitting milestones, endless meetings. I do enough of that in my day job. I can pick and choose what I want to review, when I am able. As a reviewer, there is a list of pending reviews to choose from based upon our experience and expertise in certain languages and frameworks. For instance, I often review C++, NodeJS, and React/Angular/VueJS pull requests. I tend to enjoy working on reviews for the same organization so I can pick up on the architectural design, coding style, and on-going design decisions being made...especially when I review code from a new developer joins the organization. 4) The work has been enjoyable and stress-free. I routinely do about 10-20 hours/week (in addition to my 40 hours/week day job). And it's not a fire and forget model. When I perform the review, I'm working with the developer until the review is merged. Sometimes I'll review the same piece of code multiple times as they/we work through feedback given by not only myself, but other members of the team. I am able to see comments from other PR reviewers as well as reviews from the organization. Another aspect that I haven't seen mentioned much in the comments is that we also get to review code challenges as well! Sometimes organizations don't have the expertise or time to evaluate developers. As a reviewer, I'm able to come in and provide that feedback to the organization on how the perspective candidate did. 5) The folks at PR are also really helpful. They review the reviewers and can provide good feedback, encouragement, and suggestions in order for me to provide better reviews!
• speby 5y ago
  Gosh, building any tools that target developers as customers is a gargantuan task. Kudos to you for attempting something new here and giving it a whirl. We developers are some of the harshest critics around, no matter how legit or unfounded our criticism may be. Keep it up!
• alexashka 5y ago
  What we need is software architecture review, not git diff review.
• jcadam 5y ago
  I received an invite to pullrequest nearly a year ago, filled out the application, and never heard from them again.
• racl101 5y ago
  Human linter?
• z3t4 5y ago
  > Unlimited lines. I wonder how long it will take them to go though my entire code base
• Rhodee 5y ago
  :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
  • zild3d 5y ago
    I thought of context to be more about domain/tribal knowledge rather than just "how we do things here". You can be a ruby expert, but to review a ruby PR for Stripe's backend I think you'd need to know a lot about the various internal systems, downstream users, etc that the code impacts
• game_the0ry 5y ago
  How is this not the same as contracting a freelance dev? You might as well contract through wipro / infosys / tata / cognizant / hcl. I am annoyed at how replacing software engineers is somehow viewed as a business problem that can be solved as a SaaS business or no code tooling. PR's customers need to hire more developers and pay them well. It is expensive and hard, but that is the market business people need to accept.
• ssully 5y ago
  I see a lot of comments regarding lacking context/knowledge of the full system/integration. I get that, but I think there is real value here. I have absolutely worked on teams where they were fire fighting for months at a time, where there were 1-2 developers total, and just not enough time or resources to do proper code reviews. While this service could miss big picture things, it could absolutely catch low-mid tier issues. My biggest issue would be from a security perspective. The background checks and everything are nice, but there are some systems I would just never let this service touch. Overall, this is interesting! Wish the team behind it the best of luck.
• cheriot 5y ago
  This smells like lead gen for a consulting shop.
• nn3 5y ago
  Why would anyone ever apply to a place that has such a graph on its website? https://www.pullrequest.com/images/figures/reviewers/screening-graph.png https://www.pullrequest.com/images/figures/reviewers/screeni... From a customer perspective, and if it's really true they probably will have to lower their standards to serve you too.
• siliconc0w 5y ago
  It's a bit crazy to me you'd outsource this to third party. Code reviews are the single best way spread context on a team so more than one person understands (and consents) to the change that is being made. Giving this up to 'free up engineers' for reviews seems like short term thinking. On the other hand, using this to screen hires using projects instead of inane leet code interviews seems like a good idea - especially if they worked with multiple large companies so it's many to one.
• b1nj0y 5y ago
  I think this service only applies to those open source projects, or personal learning project.