Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Code Review as a Service

pullrequest.com

Post reply on HN
Print view View original

181–190 of 238 posts

Re: Code Review as a Service

#181

Post by all2 »

avip wrote: post #179
dnautics wrote: post #141

Earlier quoted context omitted.

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.

I'd pay $$ just to have someone reviewing our Raman bowel of a codebase and documenting it.

> and documenting it

Sounds like you need two or three people dedicated to the task. Documentation is a whole profession by itself. Well, good documentation.

Re: Code Review as a Service

#182

Post by vincentmarle »

Kocrachon wrote: post #130

I understand NDAs are a thing, but I still don't think I'm too comfortable with the idea of letting a bunch of third-party people look at a bunch of my internal information. And I understand this is targeting startups a lot more than large established companies. But to me that's even more concerning because as a startup you're really trying to move fast and hoping someone doesn't beat you to the punch, and you're han…

Most startups are too busy finding product market fit than trying to worry about sharing 'secret information' (your actual secrets shouldn't be in your repo anyways).

Re: Code Review as a Service

#183

Post by Too »

ryanbrunner wrote: post #166
mncolinlee wrote: post #129

Earlier quoted context omitted.

Having done more than a few PR reviews and code security reviews for their platform as an Android/Kotlin dev, I've found that the opposite problem is more common. A lot of organizations suffer from insular thinking and their own team often comments LGTM even if there's something glaring. Writing reviews as an outsider, there's something freeing about knowing that you can review honestly and professionally and not ove…

I definitely get what you're saying here, but I think if I was given the choice between an internal reviewer that might glaze over some bad practices, or an external reviewer who will miss stuff like "oh be careful calling that code, there's gotcha X, Y, and Z that you need to think about", I'd take the former every time.

It’s usually possible to see if a certain piece of code allows gotchas or not. Global variables, implicit dependencies, undocumented apis or magic strings to give a few examples. If you have many such, then getting a reviewer calling out those bad practices is even more valuable. Even more valuable that they are external, because often many such smell-patterns are stuck due to some political stalemate or cargo-cult within the team.

Most gotchas are actually carried over from the open source framework you build on top of. Such knowledge is transferable and can’t hurt to get another pair of eyeballs to help you with them, assuming you haven’t spotted them yourself already.

Re: Code Review as a Service

#184

Post by nathanielarmer »

Jach wrote: post #170
nathanielarmer wrote: post #154

I'm a reviewer on PullRequest, and thought I'd share some perspective. Happy to answer any questions in comments. Background - I've built and led engineering teams at multiple fast-growing startups. In doing so I've seen the incredible value PR's can provide, but also the huge cost of them on small teams. I review on PullRequest part-time as I work full-time building a startup. Many of the critics here are right. The…

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 or direct resolution (great for architectural thoughts). * We can include example code snippets in our comments. This reduces the burden for the developer to adopt a certain change. I've even gone to the extent of writing small programs to validate a refactor or prove an error exists. * Sometimes it's not an error, but how that company wants X done. We take note of these for future reviewers.

Overall, this is a diplomacy game, the only power we have is soft power. I find it good practise, as I'm typically a direct kinda guy.

Results?

I've seen a lot of developers, after being introduced to new ways of working, implement that on later PR's. Sometimes immediately, sometimes after it arises a few times. Examples include improved naming, better code structure, usage of newer language features, more clarity in the code, or better SQL injection protection!

Re: Code Review as a Service

#185

Post by nathanielarmer »

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…

PullRequest actually reviews the reviewers (I actually became a better reviewer via this).

'LGTM' is considered a bad practise, and I've been pulled up on this several times. Instead, we're trained to take more time, go deeper, and be very clear about what the changes do, and how well they are written even on relatively trivial changes.

Re: Code Review as a Service

#186

Post by pictur »

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.

code review is a process that carries a lot of meaning today. the work cycle in most workplaces is not qualified to meet this meaning.

Re: Code Review as a Service

#187

Post by alistairSH »

vinceguidry wrote: post #88
alistairSH wrote: post #68

Earlier quoted context omitted.

His logic would apply to tests of all sorts. As long as the dev says the feature is done, that’s the end. And he’s wrong. As teams (or companies, or whatever unit) we get to decide our definition of “Done”. I suggest that definition include appropriate testing. And appropriate code review, which serves two purposes… catching defects and knowledge sharing.

In his world, testing the software belongs to a different team than the implementation. Differently managed, differently staffed. Not with engineers, but with testers. It's difficult to say the least to move from testing into engineering And you can't just redefine Agile like that. It's the businesses' money, culture, and productive means. Not yours. If they want teetering software stacks with no thought given to mai…

Ugh. I haven’t worked with an independent QA team in 15 years or so; testing is the responsibility of the dev team and there are test engineers on each team to facilitate that.

Maybe this is a difference of building a product vs contracting? We have to keep what we build running for years; there is no turnover to a client where can declare the system “done”.

As for redefining agile, I’ve done no such thing. The agile manifesto calls for working software. It calls for collaboration. Avoiding knowledge sharing amongst team members and throwing software over the fence to a remote test team are both counter to that goal (in my experience).

Re: Code Review as a Service

#188

Post by tyler_mann »

thewebcount wrote: post #177
tyler_mann wrote: post #70

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?

Conflict of interest is something we take seriously and have processes in place to ensure this doesn't occur. All reviewers aren't able to review or see the reviews from all customers, we have tools in place to facilitate the best matches for both compliance as well as quality and familiarity.

Re: Code Review as a Service

#189

Post by game_the0ry »

How is this not the same as contracting a freelance dev? You might as well contract through wipro / infosys / tata / cognizant / hcl.

I am annoyed at how replacing software engineers is somehow viewed as a business problem that can be solved as a SaaS business or no code tooling.

PR's customers need to hire more developers and pay them well. It is expensive and hard, but that is the market business people need to accept.

Re: Code Review as a Service

#190

Post by sbassi »

YPCrumble wrote: post #66
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…

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.

that is impossible in lot of cases
Post reply on HN