Hacker News Reader

Hacker News

Comment by rwmj | Hacker News Reader

Parent Full thread

rwmj

Agile yes, it's micromanagement at scale. But writing tests and doing code reviews is good practice.

View on HN

We used to do a kind of unit tests in place. (We called them param checking.)

In my experience unit tests have been simply a questionable yardstick management uses to feel at ease shipping code.

"98% code coverage with unit tests? Sounds good. It must be 98% bug-free — ship it."

Not that anyone ever exactly said that but that's essentially what is going on.

Code reviews seem to bring out the code-nazi types. Code reviews then break any goodwill between team members.

I preferred when I would go to a co-workers office and talk through an issue, come up with a plan to solve the problem. We trusted the other to execute.

When code reviews became a requirement it seemed to suck the joy out of being a team.

Too frequently a code review would turn into "here's how I would have implemented it therefore you're wrong, rewrite it this way, code does not pass."

Was that a shitty code reviewer? Maybe. But that's just human nature — the kind of behaviors that code "gate keeping" invites.

Everywhere I’ve worked I’ve found that the entire organization pays lip service to unit tests and code reviews, but then sets timelines so short and workloads so high that make real tests and reviews genuinely impossible.

Can echo that - it's pure lip service - the deadlines are arbitrary and change often, and nobody cares about quality beyond "can it appear to work plausibly at a demo". There are notable exceptions - nobody wants to be the next Knight Capital but a lot of work is not seen to be that critical so off to the next ticket it is.

Not in my experience. Maybe I've been lucky. Try working for bigger, established firms for better chances.

Microsoft and Facebook not large and established enough?

Yep, add Apple. (Although it varies across teams so I can only speak for my own experiences.)

Once upon a time (my career there was 26 years) code reviews, unit tests were alien. I enjoyed my job much more then.

TDD, while a good idea, is never used because it is highly unnatural when the stakes are low, which they are most of the time. And when the stakes are high enough - you'd want to hire a real QA engineer to write all the tests anyway, including the unit tests.

Code review is another sacred process that seems too good not to have, but many teams use it as a "we care about quality" stamp when in fact they do not. Used for just nitpicking code style (important but not the whole reason to have CR, and there are tools for this), issue comments like "LGTM" and approve whatever arrives at the pull request anyway.

I've not yet seen code review implemented in a good way in places I have worked. It's not really considered "real work" (may result in zero lines of code change) and it takes time to properly read through code and figuring out where the weaknesses might be. I just end up being forced to skim read for anything obvious and merging, because there is not enough time to review the code properly.

As a manager, code review has two benefits that typically matter to me: (a) cost: it's cheaper to fix a defect that hasn't shipped (reading tests for missing cases is a useful review, in my experience); (b) bus-factor: make sure someone else has a passing familiarity with the code. And some ancillary (and somewhat performative benefits) like compliance: your iso-27001, soc-2 change control processes likely require a review.

It's hard, though, to keep code reviews from turning into style and architecture reviews. Code reviewing for style is subjective. (And if someone on the team regularly produces very poor quality code, code review isn't the vehicle for fixing that.) Code reviewing for architecture is expensive; settle on a design before producing production-ready code.

My $0.02 from the other side of the manager/programmer fence.

Out of interest, how are you using code reviews to be ISO-27001 compliant?

ISO-27001's change management process requires that [you have and a execute a change management policy that requires that] changes are conducted as planned, that changes are evaluated for impact, and are authorized. In my experience, auditors will accept peer-review as a component of your change management procedure as a meaningful contributor to meeting these requirements.

"All changes are reviewed by a subject matter expert who verifies that the change meets the planned activity as described in the associated issue/ticket. Changes are not deployed to production environments until authorized by a subject matter expert after review. An independent reviewer evaluates changes for production impact before the change is deployed..."

If you are doing code review already, might as well leverage it here.

Code review where I worked seem to either in practice be rubber stamping or back scratching. Never once have I felt the need for it. If people are unsure about a change they ask usually.

If teams care about each other’s code, they ought to collaborate on its design and implementation from the start. I’ve come to see code reviews (as a gate at the end of some cycle) as an abdication of responsibility and the worst possible way to achieve alignment and high quality. Any team that gets to the end of a feature without confidence that it can be immediately rolled out to create value for users has a fundamentally flawed process.

> they ought to collaborate on its design and implementation from the start

That's exactly right. After said process, it comes down to trusting your coworkers to execute capably. And if you don't think coworker is capable, say so (or if they're junior, more prudently hand them the simpler tasks — perhaps code review behind their back and let it go if the code is "valid" — even if it is not the Best Way™ in your opinion.)

Only if there aren't QA boards with quality KPIs to fulfill, and many code reviews are wasted time in ceremonies to fulfill egos from reviewers about the only true way how to deliver software.

I usually give up, stop arguing why it is actually better than the way the gatekeepers suggest and redo my code, less time wasted.

Reply on news.ycombinator.com