Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.

...

  • Ideally everyone on the team should be familiar with all of the code (even what they aren't working on) in case the original author is busy or unavailable (or has graduated). Reviewing code improves your familiarity with the codebase, and gives you insight into why design decisions were made.
  • Code is read more often than it is written. Code reviews encourage everyone to write readable (and maintainable) code
  • Constructive feedback makes us better programmers
  • Provides visibility into the progress of each project
  • To catch bugs
  • It forces the author to reason about their code

...

Test your code, and provide your reviewers a way to see the results of your testing. Either write unit tests or provide a trace or scope capture or something..

Don't open a pull request with code that wouldn't pass your own review standards.

...

On one hand, if you're making changes to a lot of files, frequently merging into mainline means you rarely have to deal with merge conflicts, and when you do, it's significantly less painful to deal with. In addition. this forces you to think of features as incremental changes instead of dropping the entire change set all at once. I find this generally helps with modularization as well, and allows you to better scope out your work.

Linter/Formatter

Run the linter and formatter over your code before submitting a pull request. This ensures that you let a computer catch all the simple style guide violations before asking for the team's feedback.It .

But why have a style guide in the first place? When other people read the code (usually when fixing a bug), it helps to keep the code style as consistent as possible such that there isn't additional cognitive load to parse your style.

Having a linter + formatter also reduces the amount of noise that a reviewer has to point out during the pull request. If you can deal with all the small things so that reviewers can focus on the "things that matter", that's beneficial for everyone. This is why we try to have an opinionated linter, and have auto formatters such that the build fails when it doesn't meet the style guide.

Other

  • Code review is a collaborative feedback process! If you disagree with a reviewer’s comment, you should start a conversation about it, don't just ignore it, or implement it even though you disagree.

...