Versions Compared

Key

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

...

That's it! You're done. Sit back and get ready for the impending code review.

The

...

code review

We take code reviews very seriously at Cockroach Labs. Please don't be deterred if you feel like you've received some hefty feedback. That's completely normal and expected—and, if you're an external contributor, we very much appreciate your contribution!

In this repository in particular, merging a PR means accepting responsibility for maintaining that code for, quite possibly, the lifetime of CockroachDB. To take on that reponsibility, we need to ensure that meets our strict standards for production-ready codeCode reviews are one of the main ways for Cockroach Labs to guarantee quality in the product. We use them to ensure that meets our strict standards for production-ready code.

A code review is like a discussion. The reviewer can have questions, and can even sometimes request the author to write a deeper justification for changes. The goal of that discussion is to create a track record of why and how a change occurred, so we can later look back and understand the choices.

No one is expected to write perfect code on the first try. That's why we have code reviews in the first place!

...

Rarely, someone will express a sentiment like "I feel very strongly that we shouldn't merge this." Disagreements like these are often easier to resolve outside of Reviewable, via an in-person discussion or via chat.

As you gain confidence in Go, you'll find that some of the nitpicky style feedback you get does not make for obviously better code. Don't be afraid to stick to your guns and push back. Much of coding style is subjective. As I was learning Go, though, I found it helpful to try all style suggestions. For the first month, essentially every style suggestion I received made for far more idiomatic GoReviewers may also have comments about subjective matters, for example what code presentation should be considered idiomatic. When in doubt, if the approach suggested by the reviewer doesn't change functionality, it may be a learning opportunity to follow the suggestion—if the reviewer cared to give the suggestion, probably someone else would trip up in the same way too. However, remember that a review is also a two-way discussion and you should feel free to ask your own questions to the reviewer, ask them to motivate their suggestions, and you can also weigh their considerations against your own.

Once you've accumulated a set of changes you need to make to address review feedback, it's time to force-push a new revision. Be sure to amend your prior commits—you don't want to tack on a bunch of fixup commits to address review feedback. If you've split your PR into multiple commits, it can be tricky to ensure your review feedback changes make it into the right commit. A tutorial on how to do so is outside the scope of this document, but the magic keywords to Google for are "git interactive rebase."

...