Code Review as a Service • Hacker News Board

Re: Code Review as a Service

#51

Post by vinceguidry » Mon, Dec 20, 2021, 1:48 PM UTC

cannonpalms wrote: post #27

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.

I mentioned this idea to a very knowledgeable enterprise architect. I was mentioning how much I'd enjoyed being able to use the code review process in this way. His reply: As good of an idea that is, it breaks Agile. So long as the code meets the biz-provided spec, it must be accepted, and if there are other concerns with the code, to make a tech debt ticket to address it at a later date.

This, of course, horrified me.

Re: Code Review as a Service

#52

Post by senko » Mon, Dec 20, 2021, 1:49 PM UTC

cannonpalms wrote: post #27

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.

So, they find bugs and security vulnerabilities, help spot performance problems and keep the code maintainable -- as a service.

Still sounds like a pretty valuable service.

Re: Code Review as a Service

#53

Post by jermaustin1 » Mon, Dec 20, 2021, 1:53 PM UTC

andreyk wrote: post #47

alkonaut wrote: post #44

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 proje…

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. Ed…

But almost any good SA can catch all of those issues, a person doesn't need to look through the code for that.

Re: Code Review as a Service

#54

Post by senko » Mon, Dec 20, 2021, 1:54 PM UTC

booleandilemma wrote: post #45

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?

> 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

Re: Code Review as a Service

#55

Post by tialaramex » Mon, Dec 20, 2021, 1:56 PM UTC

adamnemecek wrote: post #16

hankchinaski wrote: post #7

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

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.

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)

Re: Code Review as a Service

#56

Post by swalsh » Mon, Dec 20, 2021, 1:56 PM UTC

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.

Re: Code Review as a Service

#57

Post by HPsquared » Mon, Dec 20, 2021, 1:57 PM UTC

andreyk wrote: post #47

alkonaut wrote: post #44

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 proje…

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. Ed…

What will actually happen, is people will use these kinds of "context-free code review" services, then say a code review has been done. Technically correct in a narrow way, but not what most people would expect.

Re: Code Review as a Service

#58

Post by senko » Mon, Dec 20, 2021, 1:57 PM UTC

jermaustin1 wrote: post #53

andreyk wrote: post #47

Earlier quoted context omitted.

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. Ed…

But almost any good SA can catch all of those issues, a person doesn't need to look through the code for that.

This is very close to saying Halting problem is not, in fact, a problem: https://brilliant.org/wiki/halting-problem/

Re: Code Review as a Service

#59

Post by malux85 » Mon, Dec 20, 2021, 1:58 PM UTC

andreyk wrote: post #47

alkonaut wrote: post #44

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 proje…

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. Ed…

I suspect that there’s only a tiny, tiny fraction of issues that are above the complexity that a linter and test framework can’t identify, but are below the complexity that requires deep codebase knowledge, and I would say that the overwhelming majority of value of code reviews is inside the issues that require deep codebase knowledge.

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 …

Re: Code Review as a Service

#60

Post by throwaway984393 » Mon, Dec 20, 2021, 2:01 PM UTC

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.