Live data from Hacker News

How to do a code review

google.github.io

371–376 of 376 posts

Re: How to do a code review

#371

Earlier quoted context omitted.

Some places I've worked explicitly won't push/deploy/similar on Friday.

This implies they don't have very good automated deploy, monitoring and automated rollback solution. The test of a good cd is that your ok with releasing on Friday at end of the day.

I agree I've never worked anywhere with really heavy duty CI, as in I've never worked somewhere that automated rollbacks were either trusted in all cases or business acceptable after something was production live. (Which is to say I haven't worked at places that had really significant scale.) And CI is only as good as your test coverage, which is to say if something esoteric is overlooked in your test suite...

Re: How to do a code review

#372
post #242

Here's my workflow and it works very good to get everyone on the same level: - Each night, I go over all the commits of the day and do a code review - Each morning at 9:00, we go over all the comments on the commints, with everyone, talk about it and make sure everyone understands. This allows me to explain more advanced or new concepts that one developer uses to make sure everyone understands. It allows me to introd…

This is an interesting approach. How many engineer hours do you think you spend each day doing this? If you're reviewing CLs as a group, it seems like it could get very expensive very quickly, no?

They last anywhere from 15 minutes, to 90 minutes, depending on the topics and it replaces a lot of other meetings/sharing that normally takes place.

Re: How to do a code review

#373

Earlier quoted context omitted.

Agreed. I once left a job in large part because there wasn't anyone else to review my code.

It seems incredibly self-defeating, to quit over a lack of CODE REVIEW at any point in someone's career. Hiring someone just to review your code is not a sane business decision, so you might want to think about how you overvalued that aspect. I would be very surprised if someone could make a business out of 3rd party code reviews, but stranger things have happened.

> I would be very surprised if someone could make a business out of 3rd party code reviews, but stranger things have happened.

Depending on how you define "make a business", this already happened. There are paid for code review and vulnerability scans. Sadly, I can't remember the companies that did them. I think one of them was by IBM... I saw them applied to new software (when it was nearly done) at two big, European companies. They were mostly worthless: The insights were barely above what Sonar gives you and many findings were "never gonna happen" edge cases.

Re: How to do a code review

#374
post #350
post #339

Earlier quoted context omitted.

It is! Which is why I don’t like the term. It often masks the fact that it has a cost in complexity.

i've never read it that way! could you give an example of that?

Two examples in C++: virtual method dispatch via vtable (in C you'd implement the same mechanism manually), templated generic code. C++'s generic data structures and algorithms are faster than C's because they don't have to use indirection through pointers. The compiler creates specialized code for each type instead. It causes binary size bloat, but execution time is low.

Re: How to do a code review

#375
post #99

Here's the part that resonated with me most: A particular type of complexity is over-engineering, where developers have made the code more generic than it needs to be, or added functionality that isn’t presently needed by the system. Reviewers should be especially vigilant about over-engineering. Encourage developers to solve the problem they know needs to be solved now, not the problem that the developer speculates…

When you have very smart people, it may be difficult for those people to write simple stupid code. Smart people like to write delightful, sophisticated and elegant solutions that will address problems and use cases that the code will never go through. A library may be an exception. I used to be one of them. "Growing up" I realized that striving for simplicity and adding just complexity when necessary is the ultimate…

When you have very smart people, it may be difficult for those people to write simple stupid code.

If they can't write idiomatically, are they "very smart?" Code is a communication medium as much as a functional one, and if someone doesn't understand you when you're talking to them, such that you have to say, "well I use a cutting edge grammar, aren't you familiar with Lakoffian generative semantics? Imre Lakatos has a sick presentation on Vimeo," you're not maintaining communication skills. I imagine this could be what "unshared values" means.

The way I see it, it's like driving: people who don't stop at stop signs, who don't use turn signals, who don't know right-of-way rules are not good drivers. These things are a part of driving just as much as knowing where you're going and trying to run the Nurburgring in under 8 minutes. They also demonstrate a concern and respect for the health and wellbeing of your fellow drivers (not to mention pedestrians, cyclists, etc.).

This is not a matter of education, age, or maturity, it's manners. It's saying "please," and "thank you." It's parenting yourself if your actual parents didn't teach you. I'm not sure any of that can be chalked up to surmounting ego. "Ego" is just an excuse and probably not based on actual psychological concepts anyway.

the main motivation for smart people may easily become "ego" and showing off code that may look clever and supporting a lot more use cases but most likely it did not need to be written, wasting time and adding unnecessary complexity with the risk of introducing cognitive overload.

"Very smart?" ;)

Re: How to do a code review

#376
post #350

Earlier quoted context omitted.

i've never read it that way! could you give an example of that?

Two examples in C++: virtual method dispatch via vtable (in C you'd implement the same mechanism manually), templated generic code. C++'s generic data structures and algorithms are faster than C's because they don't have to use indirection through pointers. The compiler creates specialized code for each type instead. It causes binary size bloat, but execution time is low.

so what's the "cost in complexity" here? the complexity would be there whether you went with a a hand-rolled or "zero-cost" version (at least that's the idea), except the "zero-cost" version should require less code and thus be easier to maintain.
Post reply on HN