Code Review as a Service

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.

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?

That would be lovely, but pelasco's point is still a good one. If PullRequest _only_ pays "rates[...]comparable to those of a senior-level engineer", without those extra perks (and, to be clear, I agree with you that in an ideal world those perks would not related to employment), then PR's actual total comp is actually effectively much less than for senior-level positions.

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.

As somebody who's done a bunch of reviews on the platform you realize a bunch of things about development that we don't want to admit. When I review code for my own team (for my day job), there are many times where internal pressures on my time will make me prematurely stamp LGTM on one of my co-workers PRs that I trust. When I'm doing reviews on PullRequest I remove the "trust" marker in my review along with this strange thing called "economic compensation" where I'm paid to spend time working on it, rather than having somebody ding me for not completing some other task that needs to ship this week so I spend more time reviewing the details.

I think there are a few levels of code reviews that should be in place.

* Automated checks eslint/typescript as examples, you would be surprised at how many companies don't have this!

* Best practices -- react hooks, golang interfaces, calling conventions...

* Testing -- are the tests structure to test good and bad, are they brittle? * Security -- Did you build the right IAM role in terraform, did somebody just checkin their GITHUB key (ok, that should be automated).

* Performance -- Is that Promise.all going to dispatch 1000 calls in parallel.

* Architecture and inter dependancies, there is a limit for a 3rd party here.

Now if you're the Lead/Architect on a project and at a minimum you can outsource the Best practices / Testing portion of the pull request to a 3rd party you now can focus on the architectural dependancies that are really what you care about.

As an architect you can easily provide reviewer notes to the person doing the review that you are interested in focusing on specific areas of improvement across your team. Giving you the coverage to focus on the high level issues and inter dependancies while not focusing on variable names or test coverage.

Your time is valuable, you should spend it on character development not on the punctuation.

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