Live data from Hacker News

Code Review as a Service

pullrequest.com

191–200 of 238 posts

Re: Code Review as a Service

#191
I see a lot of comments regarding lacking context/knowledge of the full system/integration. I get that, but I think there is real value here. I have absolutely worked on teams where they were fire fighting for months at a time, where there were 1-2 developers total, and just not enough time or resources to do proper code reviews. While this service could miss big picture things, it could absolutely catch low-mid tier issues.

My biggest issue would be from a security perspective. The background checks and everything are nice, but there are some systems I would just never let this service touch.

Overall, this is interesting! Wish the team behind it the best of luck.

Re: Code Review as a Service

#192

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.

Some companies doesn't have enough people to review the code, at least not with the required seniority. This way there is also knowledge transfer, but to the company. And with people not in your payroll that can provide objective comments since they are not afraid of telling the wrong person that their code sucks.

Re: Code Review as a Service

#193

Earlier quoted context omitted.

I'm talking about code where thee context is donain knowdledge and architecture. Reviewing things like style, performance, security, framework best practices etc is pretty easy work and rarely the bottleneck in a team in my experience. Basically: any kind of review where you could comment on a single file only, is easy. The important and difficult part of review is "Is this the right thing to do at all? Is it impleme…

I'll argue that, in an ideal world, most of the questions you're asking here should be addressed before anyone writes code. A basic spec, with "here's the idea", "here's how I plan to prove the concept viable", and a rough plan of "here are the software components I'll use" doesn't take long to put together, relative to actually coding the thing up. Having that document and getting it reviewed should answer a lot of…

> in an ideal world, most of the questions you're asking here should be addressed before anyone writes code.

I agree completely. But code review to me is the chance to pick up on those situations where the situation wasn’t ideal. And even if this is just one time of 100, that review was still more important than the remaining 100 “normal” reviews with more mundane feedback.

Re: Code Review as a Service

#194
post #170

Earlier quoted context omitted.

Here's a question: do you get pushback about "best practices"? (Sometimes really just "standard practices".) If so, which ones in particular? And do you actually continue trying to explain why it's a good idea, or just throw up your hands with a feeling of "I gave my advice, they can ignore it at their peril". Having actually convinced people of the value of certain practices (like favoring fast focused unit tests ov…

I've gotten pushback on best practises, especially from more junior developers or developers on a tight deadline. I do C# and one common, yet trivial example is poor naming conventions. As a reviewer we have certain tools we can use to encourage change: * We can make comments low priority, so the advice is there, but it's skippable. * We can make summary comments, that are opinionated but don't expect any immediate o…

Thanks for the reply! It's nice to hear that a lot of devs are receptive to change and aren't treating the service hostilely. I like that there's categorization with expectations encoded in the category, I can see it helping a lot of things. A huge chunk of my reviews used to be in Code Collab, which... lacked certain desirable things. (But it got right a few others, so using it wasn't a totally bad tradeoff.) One of those I wanted was useful categories -- by default you just have review comments with the only alternative being a bright red Defect comment that would block closing the review until addressed -- but it was rather annoying to all parties and so few people used it. Different conventions arose to try and address the issue but they tended to be team specific. One of my own was prefacing those low priority things with "Nitpick: " or similar like in "Nit: add space between if and (". Another convention came from being able to make file-level comments that aren't targeted at a particular line, the reviewer might preemptively check off the file if they don't expect any action and it's just broader discussion/commentary, or leave that step undone until some sort of response (maybe just clarification) or possibly broad code changes has been done.

I've sometimes had trouble convincing some older programmers about using new (or even not so new but slightly 'advanced') language features like Java lambdas or Optional or generic types (juniors/interns were often aware of and happy to use the new stuff already), to the point that for specific individuals I'd just give up and focus my review efforts on other aspects. However if I ended up touching that code myself later on, or someone else did whom I was reviewing, I'd use those newer features/encourage the other person to use them if they weren't already, and if the original person ever came back to it they'd face an argument of local consistency against trying to change it back to using the old ways. It seems like the approach of longer-term encouraging and supporting a pocket of engineers pushing for better practices would actually work with this service given the ability to make notes for future reviewers. Another flaw in Code Collab was its atrocious search which made it hard to find prior reviews about a set of files.

Re: Code Review as a Service

#195

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…

That was my initial reaction too, but then I thought "using such a service might encourage the code-based to be external-reviewer friendly?". That is, instead of all the shared history/context that internal reviewers have, make all that as explicit as possible, ideally in the code, or at least in good code comments. Would also go a long way to avoid the "bus factor" (or more commmon: the "sudden-quit factor").

No one has to die or quit. You could just hire a new dev who doesn't have to wait for someone to explain the relevant parts of the godzilla object.

Re: Code Review as a Service

#196

Earlier quoted context omitted.

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…

> Our reviewers are all highly qualified, many are maintainers of popular open source projects or work at top tech companies. Isn’t that potentially a huge problem? What if your reviewers work for my company’s competitors? I don’t want them seeing our code base. Do you have any methods to ensure that doesn’t happen?

Are you saying your employees will never leave to work for your competitors? I have not worked with this company but most of them needs an employee to sign a strong NDA that protects IP. That must be sufficient in most cases to protect your IP.

Re: Code Review as a Service

#198

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.

Even without hard stats and evidence, I guarantee most people don't think of code review as mostly being about KT. The purpose(s) of code review commonly contain the ones you described, but it varies from team to team. The most common I'd guess is putting a 2nd pair of eyes on your code, checking code quality and finding issues.

Re: Code Review as a Service

#199

Earlier quoted context omitted.

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

He's really sharp and has keen insight into how enterprise business works. His advice saves us a lot of time and money.

But if he wants you to open tech debt tickets, rather than spotting/fixing security issues immediately, this will not save the company time or money.

Re: Code Review as a Service

#200

Earlier quoted context omitted.

He's really sharp and has keen insight into how enterprise business works. His advice saves us a lot of time and money.

But if he wants you to open tech debt tickets, rather than spotting/fixing security issues immediately, this will not save the company time or money.

He doesn't. He was just describing to me the way the enterprise usually sees things. Our shop is more modern than he's used to working in.
Post reply on HN