top | item 29626215

(no title)

mncolinlee | 4 years ago

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 overly worry that a colleague might get offended when you're simply trying to help. It's also not a chore anymore. Since the review is the job itself, it doesn't feel like a distraction. And since you're an outsider, you might know about best practices at your organization that the client hasn't been exposed to.

When I review code, I read the summary explaining what that org likes in a review, but I also make sure to include tools and practices that they might not be aware of. In many cases, I can see they're lacking automated static analysis like ktlint/detekt and point it out. I might notice performance or security flaws that their own team wouldn't consider in a typical PR.

While I actually enjoyed the style of work where reviewing a PR isn't a chore, there are a couple issues I'd like to see improved. Their rates could be improved for the best engineers. Also, the number of jobs isn't always enough for the number of reviewers. Gig work is much nicer if you can actually choose the hours and have more flexibility.

discuss

order

andy_ppp|4 years ago

I think the point is internal and external code reviews are two different beasts - no harm in getting an external kicking to improve the coding practices. However, with nobody having skin in the game to get external code reviews into the codebase, they will largely be ignored as “nice but we have work to do”. How could a product like this (I think I’ve seen a few) solve that human nature problem?

mncolinlee|4 years ago

They're truly different beasts, but each has clear value. As but one example, I've seen outsourced apps for financial firms where there were literally hundreds of basic security flaws. Would you trust the same review process that allowed those PRs?

andy_ppp|4 years ago

To answer my own question, maybe you could massively overcharge and put a bounty on each review item that gets paid back when they get completed? Is it right to make something like this about money? Would the company or dev see the cash?

Could be an interesting way to make it work and try make a higher/more valuable company from this, i.e. the CRAAS company could keep the bounties if not fixed after say 6 months...

ryanbrunner|4 years ago

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.

Too|4 years ago

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.