7 ms·

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 part

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.

austinjp 9y ago
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?
- lyal 9y ago
  Ah! Bot in this case is the label applied to any application within Github.
  - austinjp 9y ago
    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.
carterehsmith 9y ago
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.
- lyal 9y ago
  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.
  - carterehsmith 9y ago
    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.
    - lyal 9y ago
      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.
kornish 9y ago
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.
- encoderer 9y ago
  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.
  - aassddffasdf 9y ago
    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).
    - kornish 9y ago
      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.
    - paulddraper 9y ago
      Side point, but AFIAK `fold()()` is most idiomatic in Scala.
      - Nullabillity 9y ago
        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.
philip1209 9y ago
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.
lyal 9y ago
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!)
- antirez 9y ago
  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!
siruncledrew 9y ago
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.

austinjp 9y ago

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?

lyal 9y ago
Ah! Bot in this case is the label applied to any application within Github.
- austinjp 9y ago
  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.

carterehsmith 9y ago

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.

lyal 9y ago
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.
- carterehsmith 9y ago
  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.
  - lyal 9y ago
    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.

kornish 9y ago

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.

encoderer 9y ago
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.
- aassddffasdf 9y ago
  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).
  - kornish 9y ago
    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.
  - paulddraper 9y ago
    Side point, but AFIAK `fold()()` is most idiomatic in Scala.
    - Nullabillity 9y ago
      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.

philip1209 9y ago

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.

lyal 9y ago

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

antirez 9y ago
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!

siruncledrew 9y ago

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.