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

No code reviews by default

raycast.com

Post reply on HN
Print view View original

281–290 of 315 posts

Re: No code reviews by default

#281

Post by mpweiher »

dragonwriter wrote: post #248
hsn915 wrote: post #222

Earlier quoted context omitted.

Books are published once, not through iterations. The equivalent in book writing would be that you are not allowed to write more than one page at a time, and someone must approve it before you start working on the next page.

> Books are published once, not through iterations. Incorrect. Bugs (“errata”) are regularly fixed in bugfix versions (“printings”) of books, whereas feature changes are regularly fixed in more major versions (“editions”). And that’s after the equivalent of a “1.0” release. Books are also regularly iteratively released either to a closed group of reviewers or even the public (e.g., Manning MEAPs, PragProg Beta Books,…

> Bugs (“errata”) are regularly fixed in bugfix versions (“printings”) of books,

But those fixes don't fix the already printed books.

Re: No code reviews by default

#282

Post by tpoacher »

Code review: the process by which a team member learns something useful from another team member (and occasionally also vice versa), by reviewing their code with them.

"We unashamedly don't do code reviews unless requested explicitly" == "We are proud to announce we don't like learning from each other unless shit hits the fan*"

* which it will

Re: No code reviews by default

#283

Post by fiala__ »

lwhi wrote: post #263

> "Every company or team is different, but one thing is the same: They all want to build the best product in the shortest time possible." Maybe this is part of the problem? Why do we need to build so incredibly quickly? Is this the only way?

It's definitely part of the problem and a big reason why software engineering is in the state it's in. To me it always seemed almost like a religion. I wish we took more from the stereotype of traditional engineering, where standards, thorough testing and reliability trump satisfying some trust fund kid founder's impatient demands.

Re: No code reviews by default

#284

Post by btschaegg »

mpweiher wrote: post #270
pbalau wrote: post #251

Earlier quoted context omitted.

Good engineers will ask for code review. Most engineers won't.

> Good engineers will ask for code review ...when they need it. Actually, really good engineers will ask to pair when they need it. Review is a distant second.

> Review is a distant second.

I agree with you there, but I'm also sure that one reason for this is that many people still see reviews as the task that only happens after everything else.

I'm convinced there's a lot of useful middle ground to be had if reviews aren't just "here's 500 LOC, please review", but maybe more… staged.

Say: Rummage around in the code a bit for yourself (ask questions if you have to), come up with a strategy of solving your problem, and submit that for review. Not in code, but maybe using markdown, preudocode, small diagrams or the like. In person (i.e. via screenshare nowadays). And if the other party doesn't find obvious holes in your strategy, then go implementing it.

I remember an interview with some Linux kernel maintainer that mentioned the same effect -- people submitting a gigantic patch for something that will simply never be merged. And all that would have been necessary to avoid this wasted work was an e-mail asking if the maintainers are at all interested in the change.

Re: No code reviews by default

#285

Post by eterm »

mpweiher wrote: post #280
alecbz wrote: post #27

Earlier quoted context omitted.

Yeah, IMO this is like a publisher saying "we don't have editors: we trust our authors". The intended ethos of editors and code review is that mistakes and imperfections are the norm, and you need a second pair of eyes to iron them out. That said, I do sometimes encounter individuals or cultures that seem to view code review more as a mechanism for catching abnormal/unexpected mistakes than as a normal part of the pr…

> this is like a publisher saying "we don't have editors: we trust our authors" No it's not. If an author produces gibberish and nobody checks, the published result is gibberish. And with print books uneditable in a run starting in the thousands. If a programmer produces gibberish, it won't compile. It also won't pass the unit test or acceptance tests, never mind QA or the alpha/beta testers.

My experience tells me there's no shortage of compilable gibberish.

Re: No code reviews by default

#286

Post by 999900000999 »

tombert wrote: post #104

I think I largely agree with this article. When I worked for Apple, we had a strict "all code needs to be reviewed" policy, which I had no problem with. Then, after being there for about 1.5 years, I make a PR, ask two different people to approve it, which they do, and it gets merged. A week later, the code is released, it looks like my code caused a fairly major bug. My manager's manager and my manager had a meeting…

That's on the QA team.

If your able to write code that screws up key functionality QA should catch it in Beta.

Unless you did something wacky like deploy straight to prod.

Re: No code reviews by default

#287

Post by mpweiher »

eterm wrote: post #285
mpweiher wrote: post #280

Earlier quoted context omitted.

> this is like a publisher saying "we don't have editors: we trust our authors" No it's not. If an author produces gibberish and nobody checks, the published result is gibberish. And with print books uneditable in a run starting in the thousands. If a programmer produces gibberish, it won't compile. It also won't pass the unit test or acceptance tests, never mind QA or the alpha/beta testers.

My experience tells me there's no shortage of compilable gibberish.

My experience tells me there's no shortage of published gibberish. ¯\_(ツ)_/¯

Re: No code reviews by default

#288

Post by northisup »

congratulations on finding engineers without egos that know when to ask for it, i've worked with a ton of great, if not over confident, people that 100% needed code review to be policy to not take the whole system down.

why i like code review:

1) code review is a great place for mentorship 2) people mess up, its ok, major issues have been caught in code review

Re: No code reviews by default

#289

Post by damethos »

tus666 wrote: post #7

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

I completely agree. This article is bad advice, please do not follow it.

Re: No code reviews by default

#290

Post by LenP »

wreath wrote: post #254
LenP wrote: post #153

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

> At my current startup, we are trying something different: the code reviewer fetches the feature branch and writes tests for the change as part of the review. As ridiculous as that sounds, it’s been working better than we’d imagined. How is this different from the "traditional" QA-writing-tests-for-devs process?

QA writing unit tests for devs is not “traditional” - it’s a signal to leave the place ASAP.

As an engineer, you’re expected to write, review, and test code. We simply shuffle responsibilities such that you usually test other engineers’ code and not your own (there is some discretion involved - for very small changes we often write or tweak the test ourselves)

Post reply on HN