Code Review as a Service
pullrequest.com
pullrequest.com
I think I might be onto something here. "Full time developers as a service"
Edit: to be clear, I personally don't see the value here, just playing devil's advocate.
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.
It's Rice's Theorem which tends to get me in trouble!
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 …
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.
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?"
Trying to review a PR for a single service that is interacting with locks across a dozen or more other services would be fraught with assumptions and missing context.
You could make the claim that if documentation is perfect, that makes the situation better for the reviewer. But this is neither a practical expectation, nor does it completely mitigate the problem.
EDIT: forgot to note that this is where bottlenecks in review are, in my experience. Not in the first-order review of syntax and semantics in the single file being reviewed.
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.
I agree completely. But code review to me is the chance to pick up on those situations where the situation wasn’t ideal. And even if this is just one time of 100, that review was still more important than the remaining 100 “normal” reviews with more mundane feedback.
If it's written "very well" then what's the point of a pull request code review? /headscratch
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.
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.
Sounds like you need two or three people dedicated to the task. Documentation is a whole profession by itself. Well, good documentation.
I'm just a reviewer and can only comment on what I've experienced. But I can say that what you're asking has definitely been done.
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?
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.
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.
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.
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.
Could be an interesting way to make it work and try make a higher/more valuable company from this, i.e. the CRAAS company could keep the bounties if not fixed after say 6 months...
Most gotchas are actually carried over from the open source framework you build on top of. Such knowledge is transferable and can’t hurt to get another pair of eyeballs to help you with them, assuming you haven’t spotted them yourself already.
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.
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.
This, of course, horrified me.
Software architecture, in this world, is how you escape the rat race of soulless ticket punching without having to go into management. You use your skills and experience in an advisory role. Obviously there are better or worse architects, I've had the pleasure of working with really good ones. But it often feels like a title and field borne out of a need to retain top talent. (read, not push them away to competitors)
For one thing, titles are just titles, and they stick around for historic reasons. For example, there really isn't a great reason for companies to call people DevOps Engineer, but it still happens.
I'll take the Solutions Architect role as an example. It's basically sales or customer-oriented developer or technical resource that works with customers to determine what a solution to a problem will look like. "Architect" is mostly meaningless in the sense that Solutions Architects don't really architect much of anything. Usually, they just come up with a plausible path forward and visualize that solution to all the stakeholders involved. This includes travel to customer sites, something that developers are basically never willing or expected to do.
They're the folks who deal with the god damn customers so the engineers don't have to. They have people skills, they're good at dealing with people. Yeah, I mean, considering the staff engineer on my team does not shower, there is value in that role.
Most of the value in the Solutions Architect is how they're able to work with customers to discover their needs on a more technical level rather than at a high level. Once the plan is determined, the solutions architects don't actually build the solution on their own, they're more like an interface or leader to the development team that builds the solution.
https://www.careerexplorer.com/careers/solution-architect/
I don't want to put words into your mouth, but maybe you're skeptical of "architects" because they aren't typically expert specialists in one small area. Maybe that's why you don't want to be associate with them. Understandable, perhaps, but don't be misguided into thinking that "architects" aren't skilled professionals who add value to the company.
On top of that, I believe they're often paid higher than developers ;-)
And you can't just redefine Agile like that. It's the businesses' money, culture, and productive means. Not yours. If they want teetering software stacks with no thought given to maintainability, managed by non-tech-savvy staff, then that's what their money will buy.
We enjoy an environment catering to our needs because our orgs can afford to throw a lot more resources at better managers and better talent. As a result individual contributors can contribute not just tickets, but also to help improve the way we work. Not possible in heavily top-down org structures.
"What parts of the system did this impact?"
"Well, this is a core piece of our ORM config, so... the entire data layer, so from a functional perspective this change impacts every part of the system"
"Then you're not changing it, QA can't rerun all their old tests, it would take months, most of those tests don't even work anymore".
End result: refactoring never happens. Get it right the first time.
It's a business decision to organize software production this way. Refactoring and ease of software maintenance are quality of life issues for engineers. Not business concerns.
Maybe this is a difference of building a product vs contracting? We have to keep what we build running for years; there is no turnover to a client where can declare the system “done”.
As for redefining agile, I’ve done no such thing. The agile manifesto calls for working software. It calls for collaboration. Avoiding knowledge sharing amongst team members and throwing software over the fence to a remote test team are both counter to that goal (in my experience).
It must be "nice" working with an enterprise architect who views agile as a strict, narrowly defined thing.
https://www.joelonsoftware.com/2000/08/09/the-joel-test-12-s...
Still sounds like a pretty valuable service.
For us the peer review is almost completely for knowledge transfer. Sure we know what each other is working on, but we still have to maintain each others code if something goes wrong and the other is not available.
So I agree with this 100%. Even in bigger organizations where the peer review was more formal, I feel like that is still the primary goal.
I do fail to see the benefit of this, especially looking at the price of it. Outside of maybe scripts? I can't really see how much they can actually help without the context of the larger application. Without that context can they really provide more benefit than AWS CodeGuru (or similar) could offer?
Simultaneously, architecture is complete mess, because architecture is much harder to see in code review.
It is, but (say it with me?): Things change.
Code review is not the sole knowledge transfer exercise, nor should it be forever. You could say similar things about "make format". Now that our formatting is automatic, we can discuss code at a higher level. If code reviews were standardized or even automated, we could discuss it at an even higher level.
That said, I think there's still a lot of space where this service can be very useful: from improving your code style and an affordable way to have security audits, to just a support net for developers who might feel overwhelmed by a task sometimes, and could use some friendly advices from a seasoned dev. It being an external service can also make it less stressful and personal, which is great as some devs see code reviews as a criticism of their skills and go all defensive about it, creating tensions in teams (sounds silly, but I've seen a lot of it).
The big thing that code review achieves is that it ensures a 2nd person understands the code that was written, and therefore it is possible for the reader to understand.
So 3 years down the road and you're looking at some counterintuitive piece of code, the reader isn't wondering *why on earth did he write it like this? Is it working around some cryptic edge-case bug in the framework or were they just stupid?"
In many places, yes.
But: When you have a solo developer working on a component in a language that you aren't familiar with, team full of novices, component delivered by a contractor...
As generic "knowledge transfer", or even increasing the bus factor, I would disagree. The best knowledge transfer experiences I've had have been dedicated meetings/workgroups dedicated to that purpose, and they tend to encompass larger scopes than a single commit/pull request. I've also seen the difference in devs who contribute to an area having previously only been reviewers of code in that area, vs. actually having a knowledge transfer session with that area's lead beforehand, and in the latter case they are more effective (actually even if they had never reviewed code in that area before, as was the case with interns or new hires). To me knowledge transfer is the nice possible side effect, but not the primary purpose.
I'll leave a third opinion from here https://static1.smartbear.co/smartbear/media/pdfs/best-kept-... which lists some direct and indirect benefits:
Few developers and software project managers would argue that these are direct benefits of conducting code reviews:
• Improved code quality
• Fewer defects in code
• Improved communication about code content
• Education of junior programmers
And these indirect benefits are byproducts of code review:
• Shorter development/test cycles
• Reduced impact on technical support
• More customer satisfaction
• More maintainable codeCode 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?
Prettier is popular for that job:
For detecting functional/idiomatic/behavioral issues, ESLint is my go-to:
This shows my bias for automation over human enforcement.
And no, you can’t write programs to do this.
Maybe due to limitations on team size, it's not possible to have an in-house expert on every technology used.
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)
At the same time, if you are fixing a critical bug and you use enum instead of boolean, who cares, just push the fix, but it doesn't hurt to have a gentle reminder show up in your github PR that it is an option to use boolean instead.
my_bills(Paid) avoids the step of reading the function prototype to find out what my_bills(true) means.
In some languages you can make piecemeal changes to upgrade from booleans because the language is happy to silently coerce between a two state Boolean and a two state enumerated type. This is probably bad news for correctness because it means my_bills(Open) might not even raise a warning but it's convenient.
Perhaps that is what this service is trying to create behind the scenes: building datasets for a high-signal-to-noise-ratio automated reviewer.
It's pretty common in larger companies to have legacy products with a couple of very senior (usually overworked) people and a larger number of junior people. The juniors very often have little experience with the language, much less the frameworks, of these legacy systems. So PR's from junior devs are often very frustrating for senior devs (wrong language conventions, not how AWS is done, paradigm misunderstandings, inaccurate comments) -- and the perverse incentives resulting from that are pretty obvious. Bad code ships.
If passing "code review as a service" were a requirement before the juniors could put up changes for seniors to review, in my experience that would be money well spent.
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.
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.
I really need to find a good mentor for our project :(
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.
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
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
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.
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?
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).
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.
SAST, or Static Application Security Testing
DAST, or Dynamic Application Security Testing
https://www.softwaresecured.com/what-do-sast-dast-iast-and-r...
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.
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.
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.
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.
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.
> 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.
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?
It's pretty clear that the people offering the service are doing it as a way of gaining extra income on the side rather than a main employment.
$50-3000 / week tells you nothing.
They should tell you the avg $ per hour.
I'd really appreciate if all companies were required to report some basics stats on pay - total employees, min/max, average, and mean would be great
The pricing on the site is also super confusing - it's $200 for one hour of reviewing, but $700 for a month? In a typical month at my job I'm doing way more than 3.5 hours of review and also doing a bunch of other stuff on the side. Then if the $700 rate is supposed to be ~120 hours then that's only $5.83/hr which isn't even minimum wage where I am. It is however on par with a lot of gig-work jobs, which makes this even more concerning.
If anyone can say what they made I think that would do a lot to quell all the people that don't trust this. I'm also sure some people must have had a bad experience on the site and I haven't seen that yet which is suspicious.
If anyone from the site happens to see this then I think you should add a breakdown of the percentage of pay going to the reviewers, or just some examples like "For C++ you can expect $40-30/hr, JS is $35-25/hr, etc"
Let people 'pay' for their own vacation.
You can provide 'profit sharing' to non-employees.
Give me $3k/week and let me manage this myself vs giving me $2.5k/week and telling me how awesome my 'health insurance' is. I want my access to health care impacted by as few third parties as possible - adding in employers to the mix is completely the wrong direction.
So, we can infer that higher-skilled individuals will take the more highly-paying positions, and that the folks working at PR will be less-skilled or juniors. That's a gross over-simplification, of course, but it probably bears consideration.
But with healthcare, at least in the US, the big issue with pricing is within the healthcare system - not employers paying for it. It certainly doesn't help to have healthcare tied to employment but having people pay high prices themselves instead of a company paying it doesn't really solve an issue it just moves it somewhere else.
It seems like the most proven solution is to socialize medical costs more, but that seems like a long-shot if we continue to insist that healthcare has to be profitable in the short-term. It's like saying "no we won't build this road because we can't charge the drivers tomorrow to make a profit on it", totally overlooking that it's an infrastructure investment and not a purchase
Thanks, I hate 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.
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?"
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).
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.
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.
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".
I do C# and one common, yet trivial example is poor naming conventions.
As a reviewer we have certain tools we can use to encourage change: * We can make comments low priority, so the advice is there, but it's skippable. * We can make summary comments, that are opinionated but don't expect any immediate or direct resolution (great for architectural thoughts). * We can include example code snippets in our comments. This reduces the burden for the developer to adopt a certain change. I've even gone to the extent of writing small programs to validate a refactor or prove an error exists. * Sometimes it's not an error, but how that company wants X done. We take note of these for future reviewers.
Overall, this is a diplomacy game, the only power we have is soft power. I find it good practise, as I'm typically a direct kinda guy.
Results?
I've seen a lot of developers, after being introduced to new ways of working, implement that on later PR's. Sometimes immediately, sometimes after it arises a few times. Examples include improved naming, better code structure, usage of newer language features, more clarity in the code, or better SQL injection protection!
I've sometimes had trouble convincing some older programmers about using new (or even not so new but slightly 'advanced') language features like Java lambdas or Optional or generic types (juniors/interns were often aware of and happy to use the new stuff already), to the point that for specific individuals I'd just give up and focus my review efforts on other aspects. However if I ended up touching that code myself later on, or someone else did whom I was reviewing, I'd use those newer features/encourage the other person to use them if they weren't already, and if the original person ever came back to it they'd face an argument of local consistency against trying to change it back to using the old ways. It seems like the approach of longer-term encouraging and supporting a pocket of engineers pushing for better practices would actually work with this service given the ability to make notes for future reviewers. Another flaw in Code Collab was its atrocious search which made it hard to find prior reviews about a set of files.
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
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.
'LGTM' is considered a bad practise, and I've been pulled up on this several times. Instead, we're trained to take more time, go deeper, and be very clear about what the changes do, and how well they are written even on relatively trivial changes.
- (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?
(In this case, they seem to be claiming Google engineers are moonlighting as reviewers, not using the service for reviewing Google’s code.)
https://techcrunch.com/2017/12/07/pullrequest-pulls-in-2-3m-...
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.
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!
I did tutoring online a while ago which claimed a lot of the same benefits, but in actuality it was hard to get a good amount of work on the site to make it worth it. You'd have to be online for hours checking the tab and then hope a 30 min session will pop up. I'm guessing there won't be that same issue here but don't want to go through the process without a general ballpark of expected income, and people being evasive in this post is kinda concerning
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.
Are they hoping to get enough training data from the consulting practice to bootstrap an AI code review product?
"Automation" = static analysis or something like https://codeql.github.com/?
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.
It's not less efficient, but is it really 2x as efficient as having a single senior dev per task, with proper code reviews and tests in place? And it does cost double upfront, you can't avoid that and costs are the limiting factor for many (especially smaller) teams... some of this cost will pay back in terms of bugs being less likely and easier future maintenance and all that, but can you really claim that it will absolutely ALWAYS return the initial investment? To paraphrase on your own words: "I’ve never seen any evidence to suggest it's guaranteed to be that much more efficient".
IME (and I did a lot of pair programming in my career) it depends a lot on the people being paired, how complementary their way of thinking is, and also on the particular task being worked on. It will never produce a worse code than solo programming, that's for sure, so it's great if you can afford it, but if you can't there are other ways around it to come close (code reviews being one), and that was the whole point of my comment.
To be sure, pairing, in order to be effective, has to be properly planned and executed. It is unlikely to work if you just leave it to your employees to figure out (unless you have some really good folk that can figure it out).
Oh, that is false. I guess it comes from a simplistic understanding of how people work (no, they are not robots). You may have two people engaged in the same task, but:
* People are more focused and work faster -- it is much easier to stop yourself from procrastinating when you have other person on the call.
* You get people exchanging tacit knowledge. With people changing pairs knowledge will tend to spread over entire team eventually very efficiently.
* You get flexibility of having any of the two people being able to continue the task (if one quits, is sick, has to take day off or step out for a meeting) -- the work is much more likely to progress uninterrupted.
* You get much less chance for the project to get stuck. When one person doesn't know how to do something the other person might know.
* You will tend to get better quality results (for example any bug needs to pass through two pairs of eyes, etc.) -- and that means less time wasted on other parts of the pipeline, less technical debt, etc.
* When you get a new dev on the team, the first assignments are much less likely to get screwed up because you have other seasoned dev in the pair.
* You get a natural mechanism to get a new dev onboarded and have knowledge transfer -- they get up to speed many, many times faster than if they are just dropped alone on a task.
* Good code review isn't free either, it would have to take a significant portion of the effort to write the code anyway.
* You get people socialising while doing useful work. Which is extra difficult while working remotely. Having tightly knit team is very valuable.
* People are overall more happy and engaged when the work goes faster. Have you noticed you feel better when you stand in one long queue that progresses fast than if you split it into multiple queues that progress slowly, even if overall wait time is the same?
* Having work done faster (by two people working on it at the same time) makes for faster cycle time and in consequence lower complexity (less things being worked on at the same time). Lowering complexity is very valuable.
* While we are at complexity -- normally doubling people in the project does not cause it to progress twice as fast or cause the throughput to be twice more. Being able to treat two developers as one, uber-developer doing work more efficiently is very valuable because it allows counteracting at least part of that diminishing returns trend (ie. you have have larger team with efficiency of a smaller team).
* As a tech lead this is fantastic way for me to get to know people, their strengths and weaknesses. I can then help them with weaknesses and maybe learn something from their strengths. Getting to know the complete picture of the team is extremely valuable.
And many other reasons.
When you take that into account you will find that, if done correctly, pair programming will be more efficient. Especially long term, because some of the effects take time to kick in.
**
Now, working in pairs isn't free:
* Working in pairs requires people to have high standards when it comes to their behaviour towards their peers. Working in pairs requires absolute adherence to the "no asshole" rule.
* As a manager, you need to react very quickly to people having trouble working together because in pair programming this is going to immediately destroy productivity for two people.
* Costs of disruption is magnified when working in pairs. For example, if you have to wait for approval for something -- now you have two discontent people waiting for the approval. If one person has to join a meeting -- the other person will not be as efficient and they will have additional cost of syncing afterwards.
* You probably want to hire people that are specifically mentally designed to be able to work in pairs. Some people just prefer to isolate themselves and work alone -- these will have trouble functioning in a team that uses pair programming. It doesn't mean these people are worse, they are just worse for that particular team.
This is to me a set of linters with human error involved..
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-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.
Or are these people working for relative poverty wages overseas?
No need to guess, it takes 20 seconds to check: https://app.pullrequest.com/signups/reviewer
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.
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.
I wonder how long it will take them to go though my entire code base
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.