Hacker News new | past | comments | ask | show | jobs | submit login

100% would recommend doing code reviews.

The most effective way I've found to do code reviews is to create a pull request for the code you would like to have reviewed. Send it out to someone or a few people that you would like to take a look, get them to approve or reject it, and then take a look at their comments and see what you can do to address them.

Some guidelines for code reviews:

1. Build each other up. The purpose of a code review is to make sure the code is good, but also to help the team grow together. If growing isn't the focus, then code reviews can quickly become harmful.

2. Get code reviewed early, often, and in small chunks. No one wants to review a 1,000 line change spanning several sub-systems, and this is the quickest way to get people to just blindly approve or to discourage them from taking the time to review. Smaller changes make it easier to justify taking 10 minutes to look over the code and make some good, constructive suggestions. It also makes it easier for the person writing the code to implement the suggestions.

3. Don't take it personally. It can be easy to let our ego get in the way when reading someone's comment about our code - after all, we might be proud of it after having spent 45 minutes on the 10 lines that someone just said "could be cleaned up"! Keep in mind that code reviews are about growing and learning, building better systems, etc.

4. The review is about the code, not the coder.

5. Honesty is key. Don't be a jerk, but don't avoid making suggestions just because you're worried it might offend someone. That just makes the review pointless!

6. Be willing spend time with the reviewer if you wrote the code, and vice-versa if you reviewed the code, to go over the suggestions made. Sometimes a comment isn't enough to adequately explain something.

I could probably write more suggestions, but I think these cover the basics.

As for tools, I just use PRs. Create one, either add certain people as reviewers or put a link to the PR somewhere that you can ask for reviewers, and then await their approval/rejection/comments. Once you have approval from the reviewers, merge it. From there you can start to build out different processes/rules around it as you see fit, but it doesn't have to be complicated and doesn't require anything fancy.

I would recommend doing them more async as opposed to scheduling "code review meetings" however. These tend to be more wasteful and can introduce a lot more stress.




This is a great list!

I'd like to suggest one if you don't mind

7. Review your own code before handing it over. Time is wasted when the submitter left out obvious issues such as typos, missing comments, unnecessary complexity, debug code, etc.

Consider the reviewer's time precious because context switching is expensive and she/he might not get back to it as quick as you'd like to.

Implementing clear guidelines and linting would help.


Using a linter goes a long way for this. Maybe make linking part of the check in process.


Seconded. Code reviews obviously helps for catching the small bugs/inefficiencies that the submitter hasn't thought of, but a surprising side-effect of having mandatory code reviews is that it actually helps everyone learn about the system as a whole, together. There's no better way to learn than to see someone (sometimes even yourself!) try something, hear that it's not correct, and see the fix. The failing and recovering steps are sometimes more helpful than just seeing "this is how we do it"-comments.

Regarding the big diffs: sometimes it's inevitable because you're doing a big formatting of the code, or you're just doing a no-op refactoring introducing new primitives. If you do so, separate commits with one being the refactoring with no functional changes, and one being the actual changes. Makes review much easier.




Consider applying for YC's Spring batch! Applications are open till Feb 11.

Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: