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

Code Review Handbook

sledgeworx.io

Post reply on HN
Print view View original

31–40 of 51 posts

Re: Code Review Handbook

#31

Post by yakshaving_jgt »

azangru wrote: post #23

> Most developers have a lot of features to add and bugs to write, time spent reviewing other people’s work does not add a lot to your bottom line. Does not add a lot to your bottom line? This is the mindset of isolated individual contributors rather than members of a single team that work on a given feature of the same product. Isn't this precisely what various agile approaches (scrum or XP in particular) are fighti…

> This is the mindset of isolated individual contributors rather than members of a single team that work on a given feature of the same product.

From experience, I think this is the more common scenario, unfortunately.

> Isn't this precisely what various agile approaches (scrum or XP in particular) are fighting against?

Maybe that was the ambition, but I don't see this borne out in practice. If anything, Agile as most commonly practiced exacerbates this problem. Of course highly paid Agile consultants will dig their heels in and say "You're not doing Agile!", but that doesn't really change the reality for the people on the ground.

Re: Code Review Handbook

#33

Post by kafrofrite »

gernb wrote: post #25
imiric wrote: post #13

It seems like a decent guide, though like others mentioned, you should download and run the code most of the time, except for trivial changes. Also, I'd like to push back on this: > Most developers have a lot of features to add and bugs to write, time spent reviewing other people’s work does not add a lot to your bottom line. I disagree; it adds a lot to your bottom line. In fact, reading and reviewing code is one of…

I went ~25yrs into my coding career without code review and without tests. I wrote games, several AAA games, play testers found bugs, but otherwise there was no code based tests and no code review, none, zero, zilch. From 1983 to around 2008. Now I use tests and code review and I like them. I like that reviewers catch my bugs. I like that they suggest better solutions I didn't think of. I like that they tell me about…

> I like that they suggest better solutions I didn't think of.

Although I don't write code full-time, when I do this is the part I enjoy more. People reviewing my code and coming up with better solutions on that same problem amazes me.

Re: Code Review Handbook

#36

Post by nicoburns »

gernb wrote: post #25
imiric wrote: post #13

It seems like a decent guide, though like others mentioned, you should download and run the code most of the time, except for trivial changes. Also, I'd like to push back on this: > Most developers have a lot of features to add and bugs to write, time spent reviewing other people’s work does not add a lot to your bottom line. I disagree; it adds a lot to your bottom line. In fact, reading and reviewing code is one of…

I went ~25yrs into my coding career without code review and without tests. I wrote games, several AAA games, play testers found bugs, but otherwise there was no code based tests and no code review, none, zero, zilch. From 1983 to around 2008. Now I use tests and code review and I like them. I like that reviewers catch my bugs. I like that they suggest better solutions I didn't think of. I like that they tell me about…

For a game that has a single release code review seems like it would be less important. Where I've found it really comes into it's own is keeping a codebase maintainable over several years.

Re: Code Review Handbook

#37

Post by infinitezest »

One of my biggest pet peeves is when people get into the habit of leaving code reviews with only suggestions like variable name changes. It's not that I'm resistant to changing things like that but I have several colleagues that seem to only ever make suggestions like that and it seems very performative to me.

Re: Code Review Handbook

#39

Post by imiric »

gernb wrote: post #25
imiric wrote: post #13

It seems like a decent guide, though like others mentioned, you should download and run the code most of the time, except for trivial changes. Also, I'd like to push back on this: > Most developers have a lot of features to add and bugs to write, time spent reviewing other people’s work does not add a lot to your bottom line. I disagree; it adds a lot to your bottom line. In fact, reading and reviewing code is one of…

I went ~25yrs into my coding career without code review and without tests. I wrote games, several AAA games, play testers found bugs, but otherwise there was no code based tests and no code review, none, zero, zilch. From 1983 to around 2008. Now I use tests and code review and I like them. I like that reviewers catch my bugs. I like that they suggest better solutions I didn't think of. I like that they tell me about…

Sure, the programming landscape was much different 30 years ago. We also didn't rely on code versioning, CI/CD, testing, and many other practices we consider standard today. The industry has since evolved to make development better and more reliable, at the expense of relative speed or simplicity. In return, we gained the ability to minimize the chances of shipping broken code that is only caught by manual testers, or, worse, actual users.

Game development particularly went through radical changes. I don't have much experience here, but I can imagine the stress of needing to ship bug-free code on actual physical media, and the excruciatingly slow dev-QA-release cycle. We're spoiled these days to have automated tests, peer review, and OTA updates, where bugs can be found, fixed, and bugfixes shipped much quicker than if we were not doing this.

So I agree that all of these processes add a mental and time overhead, but I wouldn't go back to working without them for anything.

Re: Code Review Handbook

#40

Post by teratron27 »

infinitezest wrote: post #37

One of my biggest pet peeves is when people get into the habit of leaving code reviews with only suggestions like variable name changes. It's not that I'm resistant to changing things like that but I have several colleagues that seem to only ever make suggestions like that and it seems very performative to me.

I had a colleague at a previous job who would, without fail leave reviews on only the implementation of the tests during a review. Which normally I would appreciate as testing is usually neglected but they where almost always minor nitpick style suggestions or comments (which often revealed they didn't understand the business logic / code changes being made). But as showing you proactively contributed during reviews was part of the framework for promotion they needed to show they had made suggestions rather than ask for clarification on features etc.
Post reply on HN