Live data from Hacker News

Code Review Handbook

sledgeworx.io

21–30 of 51 posts

Re: Code Review Handbook

#22
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 disagree; it adds a lot to your bottom line. In fact, reading and reviewing code is one of the main tasks of a developer, perhaps more important than writing code.

It provides the possibility of not causing future 'your business' reductions to its bottom line. That's why any 'rules' to code review are so context specific and should be applied as such.

Re: Code Review Handbook

#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 fighting against?

Re: Code Review Handbook

#25
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 functions I could use that make the code similar. I like that they help explain the code.

But, I can't entirely dismiss that I and the teams I worked on shipped a ton of products without any code review and that my velocity feels much slower than previously. It's possible that slowness is from the projects being way more complex, the teams being much larger, and that code review is actually a perf multiplier over all for these larger more complex things I'm working. But, waiting for and/or doing reviews certainly "feels" slower than I used to be.

Re: Code Review Handbook

#26
post #7

It's interesting that the author puts "downloading the code" in the exhaustive section. Is it really that much work to run git checkout? I routinely checkout code in PRs if I'm not familiar with that part of the codebase. I can navigate the codebase much quicker in my editor than in GitHub. Although GitHub is actually getting there with the semantic analysis of code and finding references and stuff, sometimes I just…

On bad projects (which happens often now that node_modules directories are often gigabytes big), IntelliJ IDEA is super-long to index, which is required before being able to Cmd+Click on functions. And we have M1. But this is an example where usability impacts the author’s judgement. For me, UX testing is part of the initial step of code reviews, no technical review is necessary if we end up deciding to implement the…

Well, if the code reviewers had correctly disallowed adding all those deps to node_modules then you wouldn't be in this situation. :P

I'm only half joking. :P

Re: Code Review Handbook

#28
post #7

It's interesting that the author puts "downloading the code" in the exhaustive section. Is it really that much work to run git checkout? I routinely checkout code in PRs if I'm not familiar with that part of the codebase. I can navigate the codebase much quicker in my editor than in GitHub. Although GitHub is actually getting there with the semantic analysis of code and finding references and stuff, sometimes I just…

I think another factor here is if the reviewer is working on the same code base and the project has a complex environment to set up (e.g., a library compiled with custom flags), then `git stash` may not be enough to checkout the branch and run tests. (I am unfortunately guilty of creating some projects like that myself...) Docker can help with these cases, but not all projects are docker-oriented.

I personally like running the code myself when reviewing, it's just not always practical.

Re: Code Review Handbook

#29
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…

> 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.

Actually, that's not true at all. Code review is not a replacement for automated builds and testing. That's what Continuous Integration is for. Let humans concentrate on what they do best (understanding and comment on code) and don't lose their time and patience with things that can be automated.

Projects often enforce that by running CI on code submitted for review and it cannot be accepted as long as a CI fails. It also reduces breakages for other devs of the builds of the code base.

Post reply on HN