Hacker News new | ask | show | jobs
by shortrounddev2 651 days ago
Code reviews to me are about, in ascending order of abstraction:

1. Code cleanliness. Stylistic issues that should ideally be handled by a linter but may get missed. Did they name their variables according to the styleguide (camelCase vs snake_case), did they use type annotations where appropriate, did they indent their code correctly.

2. Code smell. Is the dev using a hack in the language or some other bad behavior (for example, casting typescript values to unknown or any, where it can be avoided? Are they passing void* inappropriately in C/C++?)

3. Potential bugs. Are they checking for null pointers before dereferencing? Are they checking the size of an array before accessing it by index?

4. Architecture. Are they writing code in a pattern that is common to the rest of the code base? If this project is trying to use functional programming, are they introducing global state or mutability? If this project is object oriented are they using constructors correctly or are they creating factory/builder classes when appropriate? This one depends on the philosophy of the team

5. Missing work. Should they add doc comments? Should they add unit tests? Integration tests? QA information? Did they follow all the procedures for releasing code? Is the work in their code reflective of well-written requirements in the ticket?

1 comments

Solid list. We don't need to overcomplicate code review. It's a sanity test and we're all humans at the end of the day.

Give good feedback if there's any to give, and don't force it if there isn't - get the code into the codebase and just keep iterating.