Versions Compared

Old Version 25

changes.mady.by.user David Taylor

Saved on

compared with

New Version Current

changes.mady.by.user Raphael ‘kena’ Poss

Saved on

Key

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

...

  • build/ Build support scripts. You'll likely only need to interact with [build/builder.sh]. See "Building on Linux" below.

  • c-deps/ Glue to convince our build system to build non-Go dependencies. At the time of writing, "non-Go dependencies" means C or C++ dependencies. The most important of these is RocksDB, our underlying persistent key-value store.

  • cloud/kubernetes/ Kubernetes configuration to auto-launch CockroachDB clusters.

  • docs/ Documentation for CockroachDB developers. See "Internal documentation" below.

  • monitoring/ Configuration to integrate monitoring frameworks, namely Prometheus and Grafana, with CockroachDB. This configuration powers our internal monitoring dashboard as well.

  • pkg/ First-party Go code. See "Internal documentation" below for details.

  • scripts/ Handy shell scripts that aren't part of the build process. You'll likely interact with scripts/gceworker.sh most, which spins up a personal Linux VM for you to develop on in the GCE cloud.

  • vendor/ A Git submodule that contains the right version of our dependent libraries. For many years, the Go answer to dependency management was "write backwards-compatible code." That strategy is as dangerous as it sounds, so since v1.6, Go will automatically load packages in a subdirectory named vendor, if it exists. See build/README.md for details on how we maintain this folder.

...

  1. You can open a PR with an initial prototype labeled [DO NOT MERGE]. You should also assign the do-not-merge GitHub label to your PR. This is useful when you're not entirely sure who should review your code.Especially on your first PR, you can do everything short of opening a GitHub PR by sending someone a link to your local branch. Alternatively, open a PR against your fork. using the “Draft” feature. (“Create Draft PR” in the Github interface). This won't send an email notification to anyone watching this repository, so it's a good way to let your Roachmate (or the lead reviewer on your project). This works well when you have a reviewer in mind and you want to avoid the onslaught of

  2. For small changes where the approach seems obvious, you can open a PR with what you believe to be production-ready or near-production-ready code. As you get more experience with how we develop code, you'll find that larger and larger changesets will begin falling into this category.

PRs are assumed to be production-ready (option 3) unless you create it as “Draft” or say otherwise in the PR description. Be sure to note if your PR is a WIP to save your reviewer time.

...

Be kind to other developers, including your future self, by splitting large commits. Each commit should represent one logical change to the codebase with a descriptive message that explains the rationale for the change.This is explained in our PR organization philosophy.

On your first PR, it's worth re-reading the Git Commit Messages page to ensure your commit messages follow the guidelines.

...

Deciding who should review a PR requires context that you just won't have in your first month. For your starter project, your Roachmate is the obvious choice. For little bugs that you fix along the way, ask your Roachmate or your neighbor if there's someone who's obviously maintaining a particular area of the codebase. (E.g., if you had a question about the code that interfaces with RocksDBPebble, I would immediately direct you to @petermattis.)

...

If you're still unsure, just ask in chat! You'll usually get a response in under a minute.

It's important to have a tough reviewer. In the two months I've been here, I've seen a few PRs land that absolutely shouldn't have landed, each time because the person who understood why the PR was a bad idea wasn't looped into the review. There's no shame in this—it happens! I say this merely as a reminder that a tough review is a net positive, as it can save a lot of pain down the road.

...

GitHub displays the status of the latest TeamCity run at the bottom of every pull request. You can click the "Details" link to get insight into a failed build or view real-time status of an in-progress build. Occasionally, a build will fail because of flaky tests. You can verify by running a new build and seeing if the problem disappears; just hit the "Run" button on the top right of the page GitHub links to queue a new build. Less frequently, TeamCity will entirely fail to notice a PR, and GitHub will display "waiting for status to be reported" forever. The "TeamCity Continuous Integration" wiki page describes how to fix this.

...

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

...

  • CR, "code review"

  • PR, "pull request"—you probably knew that one.

  • PTAL, "please take another look"

  • RFAL, "ready for another look"—as in, I made some changes, PTAL.

  • LGTM, "looks good to me"—i.e., ship it!

  • woman_scientistImage Modifieddog

    , "science dog"—i.e., I have no idea what this code does.

  • TF[YT]R, "thanks for your/the review"

...