Earlier quoted context omitted.
Exactly. I want my code peer reviewed partly because it makes it clear and formal that while we succeed as a team we also fail as a team. It’s much easier to talk about failures when a failure doesn’t have a single person attached to it.
> I want my code peer reviewed TFA: > Engineers [..] request reviews when they think it's necessary. Problemo solved.
No code reviews by default
251–260 of 315 posts
Re: No code reviews by default
#252I much prefer (software) design reviews, they make code reviews a sort of a sanity check to see if the design was followed (reasonably), we're testing the right thing, and maybe there were some business rules that weren't followed because they weren't obvious in the design process. This is especially important when you have a lot of engineers in the team/organization.
Clicking around in Raycast's jobs page, I get the impression they prefer ICs to work individually and ship things on their own with little to no collaboration, so no code reviews seems to be aligned with their values.
Re: No code reviews by default
#253Earlier quoted context omitted.
Exactly. I want my code peer reviewed partly because it makes it clear and formal that while we succeed as a team we also fail as a team. It’s much easier to talk about failures when a failure doesn’t have a single person attached to it.
> I want my code peer reviewed TFA: > Engineers [..] request reviews when they think it's necessary. Problemo solved.
Re: No code reviews by default
#254Interesting read, and it may be working well for Raycast, but it didn’t resonate with me. The main drawback of code reviews, as they correctly note, is a lack of velocity due to engineers looking at code review as a “not my real work” thing. That’s a cultural issue in my opinion. At my current startup, we are trying something different: the code reviewer fetches the feature branch and writes tests for the change as p…
How is this different from the "traditional" QA-writing-tests-for-devs process?
Re: No code reviews by default
#255This is a remarkably frustrating read. Of the value we get from code review, catching bugs is one of the smallest. Things we catch include - Duplicating functionality we have elsewhere. - Duplicating business logic we are elsewhere. - Recommending cleaner abstractions, particularly with younger devs. - Query optimization - The time for your DBA/data developer to review things is well before they are a problem. Develo…
This is especially important when you have people otherwise working in silos, which seems to be the case for Raycast.
Re: No code reviews by default
#256Earlier quoted context omitted.
Exactly. I want my code peer reviewed partly because it makes it clear and formal that while we succeed as a team we also fail as a team. It’s much easier to talk about failures when a failure doesn’t have a single person attached to it.
> I want my code peer reviewed TFA: > Engineers [..] request reviews when they think it's necessary. Problemo solved.
Re: No code reviews by default
#257Earlier quoted context omitted.
For many developers, people merge their branches directly to production so the pull request review is where that checking happens.
Sure, I mostly meant automated processes. They won't always save you from yourself but doing blue-green, having an extensive test suite, etc, are all things that will help reduce the risk of a bad deployment.
Re: No code reviews by default
#258Earlier quoted context omitted.
It's the same in the financial sector, you can't just push code without a review. However, the review process is far from perfect and can create a false sense of security. To review a piece of code, that code should ideally be small in scope. For larger pieces it's rather common that the reviewer don't have enough time to do a good job. If there were better incentives for doing a review, then it would greatly improve…
I've had a lot of trouble getting people to do proper reviews. A minimum to me is that you actually compile and execute the code in some way to check its sane. Better would be the reviewer actually adds to the test suite for the code to prove their expectations of how it works. In almost all cases it's very hard to get people to look outside the web browser for the diffs. Diffs show you something, but never the whole…
The main purpose of code reviews is checking the general structure of the code and tests. Is the code well designed? Are error cases considered and tested? Another important purpose of code reviews is knowledge sharing.
Re: No code reviews by default
#259Re: No code reviews by default
#260He mentions "trust" a lot. Is that why we do code reviews, because we don't trust each other? I find this attitude problematic. We do code reviews because humans make mistakes. Requirements can be misinterpreted. Different work in progress can be in conflict with each other. Reviews are a good way to learn from each other and keep abreast of what work is occurring outside your own bubble. Not doing code reviews becau…
That final, 1 word sentence is one of the major problems of PRs. An engineer might spend several hours really thinking through a problem, talking with colleagues, whiteboarding options and coming up with a workable solution that addresses all the obvious issues and a bunch of non-obvious ones. Only for a drive-by take-down by someone with none of the context. It's a totally asymmetric investment of time. Even helpful…