Code Review as a Service • Hacker News Board

Re: Code Review as a Service

#141

Post by dnautics » Mon, Dec 20, 2021, 4:22 PM UTC

tyler_mann wrote: post #70

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…

Disclaimer, I'm the CTO @ PullRequest. 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…

I wonder if, instead of just incremental code reviews, there would also be a way to get a 3rd party review our huge codebase and flag issues (architectural, real legibility -- not just "CC measures") to be dealt with. Then you could keep track of them and burn them down as part of "killing technical debt" goals.

Re: Code Review as a Service

#142

Post by coryrc » Mon, Dec 20, 2021, 4:24 PM UTC

blacklion wrote: post #90

YPCrumble wrote: post #66

Earlier quoted context omitted.

Code should be written to be understood without that context. If comments and documentation aren’t enough context for the code to be understood, it probably isn’t written very well.

It is very bold statement. Code models some real-world entities (domain). Code (completely with comments and documentation) cannot and should not document fully domain. It is context, which is needed to understand code. Yes, simple CRUD application can have all context encapsulated, but what's about some code which models, say, some aspect of chemistry? Should this code have enough context which includes several post…

Your example is, IMO, the exact use of this service. If you're a chemistry expert, you're probably not a coding one. These reviewers will ensure your code is testable and likely to do what you hope it does, in a way where your fellow experts can read and write their own automated proofs (tests).

Re: Code Review as a Service

#143

Post by dangus » Mon, Dec 20, 2021, 4:24 PM UTC

haggy102 wrote: post #64

vinceguidry wrote: post #51

Earlier quoted context omitted.

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

Anyone with Architect in their title receives skepticism from me. I've turned down title changes that include it and I refuse to put it on my LinkedIn.

I think that attitude is a little short-sighted, and perhaps a little bit of "anyone that isn't a pure developer is incompetent" elitism.

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

Re: Code Review as a Service

#144

Post by skeeter2020 » Mon, Dec 20, 2021, 4:28 PM UTC

This reminds of the first (and only) time I had a full 360 degree review: contradictory, zero-context opinions from people I don't know very well who haven't earned the right to interact with me in a way required for this minefield of feedback.

Re: Code Review as a Service

#145

Post by darkstar999 » Mon, Dec 20, 2021, 4:30 PM UTC

seneca wrote: post #131

This post has a number of what look like astroturf advertising comments.

That's the word I was looking for. It really does look like that.

Re: Code Review as a Service

#146

Post by skeeter2020 » Mon, Dec 20, 2021, 4:33 PM UTC

vinceguidry wrote: post #51

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

>> it breaks Agile

It must be "nice" working with an enterprise architect who views agile as a strict, narrowly defined thing.

Re: Code Review as a Service

#147

Post by darkstar999 » Mon, Dec 20, 2021, 4:33 PM UTC

dawnerd wrote: post #139

I had to flag this after seeing several obvious spam replies trying to boost the service. Come on now.

The many multi-paragraph praises from green accounts did it for me.

Re: Code Review as a Service

#148

Post by MajorBee » Mon, Dec 20, 2021, 4:33 PM UTC

dhimes wrote: post #96

napolux wrote: post #77

LOL. This will become in less than 1 month a "LGTM" feast. 3rd world countries developers (organizing themselves in groups to pass the intro test from the website) giving green lights to random people around the world for peanuts. This is a clever business model if you ask me. Let's be clear for a second. Nobody, not event the legendary 10x developers can review properly code without context or everything mentioned h…

Could you clarify what you mean by "LGTM?" Just people saying, "looks good to me" and doing crappy reviews, or is there some LGTM "thing" out there that everybody will use? Or something else?

There is a meme out there in the wild (or at least, I think there is a meme) that many reviewers would simply approve PRs blindly leaving a LGTM (looks good to me) comment when their internal monologue is closer to "...I have no idea what's going on here and I'm too afraid to ask at this point". I suppose it could also be sheer laziness of not wanting to parse many lines of new code, especially if you're in a situation where you don't have a 100% tight understanding of the codebase (as I have been in many a times).

Re: Code Review as a Service

#149

Post by akhil-ghatiki » Mon, Dec 20, 2021, 4:34 PM UTC

How does this play with the Non disclosure agreements ?

Re: Code Review as a Service

#150

Post by niros_valtos » Mon, Dec 20, 2021, 4:37 PM UTC

rwmj wrote: post #24

niros_valtos wrote: post #6

Saw this link here in the past. Didn’t pickup. The reason why I don’t like it is that random people, regardless their expertise, cannot just review PRs and understand the impact of the change on the system without being deeply involved in the product.

I'm a little more positive about this. A careful code reviewer might be able to spot generic security or even logic flaws in code, such as inappropriate use of strcpy in a C program or code which is unreachable in a way that cannot be detected by the compiler. Also there's a lot of scope for automation, such as running Coverity or other free and commercial linters/checkers, although also a danger of overwhelming the…

Results from these tools typically go to the developers, not to the PR reviewers. They see only the result of fixing a vulnerability, and if there is a new vulnerability introduced, these static code analysis tools should detected them before deployment to dev/prod.