Hacker News Reader

Comment by antirez | Hacker News Reader

I'm not sure what the intended audience for this is exactly, but I know for sure that in certain software that have an high degree of complexity and moving parts, to see if the PR is sane you need to be an expert of that code base. No way to make "local" decisions. (cit. Butterfly Effect)

Maybe for things like mobile applications or React or stuff like that, where there are many patterns to follow, it simpler to see if something may be wrong, but I still expect an high degree of false negatives.

View on HN

Hmmm so how do you scale this business of reviewing code?

At the beginning, maybe you sign up some super high-end 10X developers to do reviews. Also run some tools like SpotBugs or Coverity or something.

Then when the ammount of reviews goes up by 10X, what are you going to do? You can't possibly find many 10X developers that are willing to review random crap code.

So you end up reselling cloud-based Coverity licenses (SpotBugs is free).

Also, this only works if the company is willing to share its code with a random startup. Which is kind of a no-go for large shops. So who do you sell to? Large shops won't allow access to their code. Startups would, but they either won't pay in principle (hey we can use SpotBugs or Coverity) or won't pay much.

Can you make a business of it? I don't know, but there are some investments being made, so I guess there is a better story than the one I see.

Disclosure: I'm Lyal Avery, founder of PullRequest.

On functional terms, we have a couple of thousand reviewers that have signed up to review. Our tooling helps them work faster; long term, we'll also provide this tooling to internal terms.

So far, we have about a dozen of the Fortune 500 signed up; larger teams, for a variety of reasons, are more open in some cases (one of the many learnings of this startup).

Edit: signed not signing.

First off, I wish you all the best.

"we have about a dozen of the Fortune 500 signing up"

Let's be honest. What this means is that someone in the Fortune 500 clicked on your email advertisement. Maybe installed your software.

Do you have anyone in Fortune 500 running critical business on your platform?

> we have a couple of thousand reviewers that have signed up to review.

Anyone can sign up a couple thousand "Mechanical Turk" reviewers in no time. Or "RentACoder" or whatever. What is the value provided? Where I work, the value of "review" provided by someone not deeply involved in the code is "0". Which is why you ask some relevant people to review the code.

Yes; we most definitely have teams from Fortune 500s running mission critical code through out platform. We will be releasing a case study from one in the next quarter.

I would say the difference on the reviewer side of our platform is in quality. We are working to surface this more.

Indeed. I'm really curious about the example I saw on their front page, apparently showing a bot commenting that "accessing this ImageView outside the main thread may cause crashes in this app". Is this genuinely possible to achieve?

Ah! Bot in this case is the label applied to any application within Github.

Okay didn't realise that, thanks. So it's humans. Well, yes, in that case I fully agree with others' comments about sustainability and scalability.

Definitely a concern within the business -- we're trying to solve this through a combination of tooling and reassignment of reviewers to the same projects. Within tooling, we use analysis to pair developers with expertise against a project (if you've built a middleware stack in python ten times, it becomes easier to gain context and identify issues).

Totally true in places like mobile/react, or projects where teams are unlikely to have significant internal expertise despite technology in production (see a midsized game studio running an erlang server for chat) providing value is easy. We believe our approach is working, and will continue to get better.

(On a personal level -- huge fan of your work!)

Thanks for replying directly! Btw what I said is definitely not a limit for your business IMHO, because even if you continue to have an high rate of false negatives, many businesses may find valuable to find some bugs using your service: to lower the amount of imperfections can be already good enough. After all no code review is able to spot any bug. It's just that in complex software with many moving parts, one should understand that probably two kinds of reviews are needed, more "local" reviews that can still spot certain bugs, and other conceptual reviews made by people which are very expert in the code base. Moreover, I think that for large customers, you may even have a product that involves developers becoming experts of a single code base of a large customer to provide in-depth reviews. Good luck!

Totally. I'm also curious about consistency: if you get reviewers who prefer different idioms and stylistic points between review, what does that do for codebase style over time? I wonder how their quality control operates on the reviewer side.

We both internally and have customer rate reviews -- we also establish corpus details with customers during the onboarding process. On the whole, we believe it's valuable to stay within the structure established within a team and nudge things forward rather than making wholesale changes to disrupt.

If that’s what you’re covering or expecting from code reviews you’re doing it wrong.

The only right answer here is automated code formatting and linting.

That is not what is meant by "idioms and stylistic aspects". Take, e.g. `o.map(v => ...).getOrElse(v2)` vs `o match case Some(v) => ... case None => v2`. The former is idiomatic and preferred over the latter (but of course some jackass somewhere will disagree and argue incessantly about it).

Exactly right. I'm not referring to which line the braces go on, but rather how domains get modeled, how errors are handled, how functional or imperative the code is, etc.

Side point, but AFIAK `fold()()` is most idiomatic in Scala.

And yet, `match` is the only one that gets correct type inference.

`.map(ifSome).getOrElse(ifNone)` infers the type from `ifSome`, and forces `ifNone` to conform.

`.fold(ifNone)(ifSome)` infers the type from `ifNone`, and forces `ifSome` to conform.

Meanwhile, `match { case Some(x) => ifSome(x); case None(x) => ifNone }` will correctly infer from both cases at once, and give you the greatest common ancestor.

Yeah, I agree. ¯\_(ツ)_/¯

I think a big use case could be letting non-technical project owners get insight into whether their project is on-track and good. Really important for people who outsource projects on Upwork or something.

However, it does seem like the bigger need is the right code the first time. Of course, that's impossible, but this seems like a secondary need rather than the primary issue.

Exactly. It may take a ton of time to properly review code in certain complex cases, which would be super costly. That kind of specialization seems too much for an average "Code Review as a Service" engagement.

Aside from that, I'm not sure if the employee developers will get annoyed with having a third-party come in and "tell them how to do things" or potentially mistakenly point out non-issues. I see why management may want to do this, but I wonder whether the developers will be pleased having a third-party put on their turf.

Reply on news.ycombinator.com