Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 1 | # Contribution Guide |
Marcel van Lohuizen | 0949135 | 2018-12-20 20:20:54 +0100 | [diff] [blame] | 2 | |
Marcel van Lohuizen | 0949135 | 2018-12-20 20:20:54 +0100 | [diff] [blame] | 3 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 4 | The CUE project welcomes all contributors. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 5 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 6 | This document is a guide to help you through the process |
| 7 | of contributing to the CUE project, which is a little different |
| 8 | from that used by other open source projects. |
| 9 | We assume you have a basic understanding of Git and Go. |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 10 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 11 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 12 | ## Becoming a contributor |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 13 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 14 | ### Overview |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 15 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 16 | The first step is registering as a CUE contributor and configuring your environment. |
| 17 | Here is a checklist of the required steps to follow: |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 18 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 19 | |
Marcel van Lohuizen | 3c3de28 | 2018-12-20 21:31:08 +0100 | [diff] [blame] | 20 | - **Step 0**: Decide on a single Google Account you will be using to contribute to CUE. |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 21 | Use that account for all the following steps and make sure that `git` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 22 | is configured to create commits with that account's e-mail address. |
Marcel van Lohuizen | 3c3de28 | 2018-12-20 21:31:08 +0100 | [diff] [blame] | 23 | - **Step 1**: [Sign and submit](https://cla.developers.google.com/clas) a |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 24 | CLA (Contributor License Agreement). |
Marcel van Lohuizen | 3c3de28 | 2018-12-20 21:31:08 +0100 | [diff] [blame] | 25 | - **Step 2**: Configure authentication credentials for the CUE Git repository. |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 26 | Visit |
| 27 | [cue.googlesource.com](https://cue.googlesource.com), click |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 28 | on "Generate Password" (top right), and follow the instructions. |
Marcel van Lohuizen | 3c3de28 | 2018-12-20 21:31:08 +0100 | [diff] [blame] | 29 | - **Step 3**: Register for Gerrit, the code review tool used by the CUE team, |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 30 | by [visiting this page](https://cue-review.googlesource.com/login/). |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 31 | The CLA and the registration need to be done only once for your account. |
Marcel van Lohuizen | 3c3de28 | 2018-12-20 21:31:08 +0100 | [diff] [blame] | 32 | - **Step 4**: Install `git-codereview` by running |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 33 | `go get -u golang.org/x/review/git-codereview` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 34 | |
| 35 | <!-- TODO |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 36 | If you prefer, there is an automated tool that walks through these steps. |
| 37 | Just run: |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 38 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 39 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 40 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 41 | $ go get -u cuelang.org/x/tools/cmd/cue-contrib-init |
| 42 | $ cd /code/to/edit |
| 43 | $ cue-contrib-init |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 44 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 45 | ---> |
| 46 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 47 | The rest of this chapter elaborates on these instructions. |
| 48 | If you have completed the steps above (either manually or through the tool), jump to |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 49 | Before contributing code. |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 50 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 51 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 52 | ### Step 0: Select a Google Account |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 53 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 54 | A contribution to CUE is made through a Google account with a specific |
| 55 | e-mail address. |
| 56 | Make sure to use the same account throughout the process and |
| 57 | for all your subsequent contributions. |
| 58 | You may need to decide whether to use a personal address or a corporate address. |
| 59 | The choice will depend on who |
| 60 | will own the copyright for the code that you will be writing |
| 61 | and submitting. |
| 62 | You might want to discuss this topic with your employer before deciding which |
| 63 | account to use. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 64 | |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 65 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 66 | Google accounts can either be Gmail e-mail accounts, G Suite organization accounts, or |
| 67 | accounts associated with an external e-mail address. |
| 68 | For instance, if you need to use |
| 69 | an existing corporate e-mail that is not managed through G Suite, you can create |
| 70 | an account associated |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 71 | [with your existing |
| 72 | e-mail address](https://accounts.google.com/SignUpWithoutGmail). |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 73 | |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 74 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 75 | You also need to make sure that your Git tool is configured to create commits |
| 76 | using your chosen e-mail address. |
| 77 | You can either configure Git globally |
| 78 | (as a default for all projects), or locally (for a single specific project). |
| 79 | You can check the current configuration with this command: |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 80 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 81 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 82 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 83 | $ git config --global user.email # check current global config |
| 84 | $ git config user.email # check current local config |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 85 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 86 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 87 | To change the configured address: |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 88 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 89 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 90 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 91 | $ git config --global user.email name@example.com # change global config |
| 92 | $ git config user.email name@example.com # change local config |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 93 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 94 | |
| 95 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 96 | ### Step 1: Contributor License Agreement |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 97 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 98 | Before sending your first change to the CUE project |
| 99 | you must have completed one of the following two CLAs. |
| 100 | Which CLA you should sign depends on who owns the copyright to your work. |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 101 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 102 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 103 | - If you are the copyright holder, you will need to agree to the |
| 104 | [individual contributor license agreement](https://developers.google.com/open-source/cla/individual), |
| 105 | which can be completed online. |
| 106 | - If your organization is the copyright holder, the organization |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 107 | will need to agree to the |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 108 | [corporate |
| 109 | contributor license agreement](https://developers.google.com/open-source/cla/corporate). |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 110 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 111 | You can check your currently signed agreements and sign new ones at |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 112 | the |
| 113 | [Google Developers Contributor License Agreements](https://cla.developers.google.com/clas?pli=1&authuser=1) website. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 114 | If the copyright holder for your contribution has already completed the |
| 115 | agreement in connection with another Google open source project, |
| 116 | it does not need to be completed again. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 117 | |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 118 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 119 | If the copyright holder for the code you are submitting changes—for example, |
| 120 | if you start contributing code on behalf of a new company—please send mail |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 121 | to the [`cue-dev` mailing list](mailto:cue-dev@googlegroups.com). |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 122 | This will let us know the situation so we can make sure an appropriate agreement is |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 123 | completed and update the `AUTHORS` file. |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 124 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 125 | |
| 126 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 127 | ### Step 2: Configure git authentication |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 128 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 129 | The main CUE repository is located at |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 130 | [cue.googlesource.com](https://cue.googlesource.com), |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 131 | a Git server hosted by Google. |
| 132 | Authentication on the web server is made through your Google account, but |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 133 | you also need to configure `git` on your computer to access it. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 134 | Follow this steps: |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 135 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 136 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 137 | - Visit [cue.googlesource.com](https://cue.googlesource.com) |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 138 | and click on "Generate Password" in the page's top right menu bar. |
| 139 | You will be redirected to accounts.google.com to sign in. |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 140 | - After signing in, you will be taken to a page with the title "Configure Git". |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 141 | This page contains a personalized script that when run locally will configure Git |
| 142 | to hold your unique authentication key. |
| 143 | This key is paired with one that is generated and stored on the server, |
| 144 | analogous to how SSH keys work. |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 145 | - Copy and run this script locally in your terminal to store your secret |
| 146 | authentication token in a `.gitcookies` file. |
| 147 | If you are using a Windows computer and running `cmd`, |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 148 | you should instead follow the instructions in the yellow box to run the command; |
| 149 | otherwise run the regular script. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 150 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 151 | ### Step 3: Create a Gerrit account |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 152 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 153 | Gerrit is an open-source tool used by CUE maintainers to discuss and review |
| 154 | code submissions. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 155 | |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 156 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 157 | To register your account, visit |
| 158 | [cue-review.googlesource.com/login/](https://cue-review.googlesource.com/login/) |
| 159 | and sign in once using the same Google Account you used above. |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 160 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 161 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 162 | ### Step 4: Install the git-codereview command |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 163 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 164 | Changes to CUE must be reviewed before they are accepted, no matter who makes the change. |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 165 | A custom `git` command called `git-codereview` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 166 | simplifies sending changes to Gerrit. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 167 | |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 168 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 169 | Install the `git-codereview` command by running, |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 170 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 171 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 172 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 173 | $ go get -u golang.org/x/review/git-codereview |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 174 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 175 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 176 | Make sure `git-codereview` is installed in your shell path, so that the |
| 177 | `git` command can find it. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 178 | Check that |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 179 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 180 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 181 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 182 | $ git codereview help |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 183 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 184 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 185 | prints help text, not an error. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 186 | |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 187 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 188 | On Windows, when using git-bash you must make sure that |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 189 | `git-codereview.exe` is in your `git` exec-path. |
| 190 | Run `git --exec-path` to discover the right location then create a |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 191 | symbolic link or just copy the executable from $GOPATH/bin to this directory. |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 192 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 193 | |
| 194 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 195 | ## Before contributing code |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 196 | |
| 197 | <!-- |
| 198 | TODO |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 199 | The project welcomes code patches, but to make sure things are well |
| 200 | coordinated you should discuss any significant change before starting |
| 201 | the work. |
| 202 | It's recommended that you signal your intention to contribute in the |
| 203 | issue tracker, either by <a href="https://cuelang.org/issue/new">filing |
| 204 | a new issue</a> or by claiming |
| 205 | an <a href="https://cuelang.org/issues">existing one</a>. |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 206 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 207 | --> |
| 208 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 209 | ### Check the issue tracker |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 210 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 211 | Whether you already know what contribution to make, or you are searching for |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 212 | an idea, the [issue tracker](https://github.com/cuelang/cue/issues) is |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 213 | always the first place to go. |
| 214 | Issues are triaged to categorize them and manage the workflow. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 215 | |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 216 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 217 | Most issues will be marked with one of the following workflow labels: |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 218 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 219 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 220 | - **NeedsInvestigation**: The issue is not fully understood |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 221 | and requires analysis to understand the root cause. |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 222 | - **NeedsDecision**: the issue is relatively well understood, but the |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 223 | CUE team hasn't yet decided the best way to address it. |
| 224 | It would be better to wait for a decision before writing code. |
| 225 | If you are interested on working on an issue in this state, |
| 226 | feel free to "ping" maintainers in the issue's comments |
| 227 | if some time has passed without a decision. |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 228 | - **NeedsFix**: the issue is fully understood and code can be written |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 229 | to fix it. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 230 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 231 | You can use GitHub's search functionality to find issues to help out with. Examples: |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 232 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 233 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 234 | - Issues that need investigation: |
| 235 | [`is:issue is:open label:NeedsInvestigation`]( |
| 236 | https://github.com/cuelang/cue/issues?q=is%3Aissue+is%3Aopen+label%3ANeedsInvestigation) |
| 237 | - Issues that need a fix: |
| 238 | [`is:issue is:open label:NeedsFix`](https://github.com/cuelang/cue/issues?q=is%3Aissue+is%3Aopen+label%3ANeedsFix) |
| 239 | - Issues that need a fix and have a CL: |
| 240 | [`is:issue is:open label:NeedsFix "cuelang.org/cl"`](https://github.com/cuelang/cue/issues?q=is%3Aissue+is%3Aopen+label%3ANeedsFix+%22golang.org%2Fcl%22) |
| 241 | - Issues that need a fix and do not have a CL: |
| 242 | [`is:issue is:open label:NeedsFix NOT "cuelang.org/cl"`](https://github.com/cuelang/cue/issues?q=is%3Aissue+is%3Aopen+label%3ANeedsFix+NOT+%22golang.org%2Fcl%22) |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 243 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 244 | ### Open an issue for any new problem |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 245 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 246 | Excluding very trivial changes, all contributions should be connected |
| 247 | to an existing issue. |
| 248 | Feel free to open one and discuss your plans. |
| 249 | This process gives everyone a chance to validate the design, |
| 250 | helps prevent duplication of effort, |
| 251 | and ensures that the idea fits inside the goals for the language and tools. |
| 252 | It also checks that the design is sound before code is written; |
| 253 | the code review tool is not the place for high-level discussions. |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 254 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 255 | |
| 256 | <!-- |
| 257 | TODO |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 258 | When planning work, please note that the CUE project follows a <a |
| 259 | href="https://cuelang.org/wiki/CUE-Release-Cycle">six-month development cycle</a>. |
| 260 | The latter half of each cycle is a three-month feature freeze during |
| 261 | which only bug fixes and documentation updates are accepted. |
| 262 | New contributions can be sent during a feature freeze, but they will |
| 263 | not be merged until the freeze is over. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 264 | |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 265 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 266 | Significant changes to the language, libraries, or tools must go |
| 267 | through the |
| 268 | <a href="https://cuelang.org/s/proposal-process">change proposal process</a> |
| 269 | before they can be accepted. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 270 | |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 271 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 272 | Sensitive security-related issues (only!) should be reported to <a href="mailto:security@cuelang.org">security@cuelang.org</a>. |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 273 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 274 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 275 | ## Sending a change via GitHub |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 276 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 277 | First-time contributors that are already familiar with the |
| 278 | <a href="https://guides.github.com/introduction/flow/">GitHub flow</a> |
| 279 | are encouraged to use the same process for CUE contributions. |
| 280 | Even though CUE |
| 281 | maintainers use Gerrit for code review, a bot called Gopherbot has been created to sync |
| 282 | GitHub pull requests to Gerrit. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 283 | |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 284 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 285 | Open a pull request as you normally would. |
| 286 | Gopherbot will create a corresponding Gerrit change and post a link to |
| 287 | it on your GitHub pull request; updates to the pull request will also |
| 288 | get reflected in the Gerrit change. |
| 289 | When somebody comments on the change, their comment will be also |
| 290 | posted in your pull request, so you will get a notification. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 291 | |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 292 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 293 | Some things to keep in mind: |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 294 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 295 | |
| 296 | <ul> |
| 297 | <li> |
| 298 | To update the pull request with new code, just push it to the branch; you can either |
| 299 | add more commits, or rebase and force-push (both styles are accepted). |
| 300 | </li> |
| 301 | <li> |
| 302 | If the request is accepted, all commits will be squashed, and the final |
| 303 | commit description will be composed by concatenating the pull request's |
| 304 | title and description. |
| 305 | The individual commits' descriptions will be discarded. |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 306 | See Writing good commit messages</a> for some |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 307 | suggestions. |
| 308 | </li> |
| 309 | <li> |
| 310 | Gopherbot is unable to sync line-by-line codereview into GitHub: only the |
| 311 | contents of the overall comment on the request will be synced. |
| 312 | Remember you can always visit Gerrit to see the fine-grained review. |
| 313 | </li> |
| 314 | </ul> |
| 315 | --> |
| 316 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 317 | ## Sending a change via Gerrit |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 318 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 319 | It is not possible to fully sync Gerrit and GitHub, at least at the moment, |
| 320 | so we recommend learning Gerrit. |
| 321 | It's different but powerful and familiarity |
| 322 | with help you understand the flow. |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 323 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 324 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 325 | ### Overview |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 326 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 327 | This is an overview of the overall process: |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 328 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 329 | |
Marcel van Lohuizen | 3c3de28 | 2018-12-20 21:31:08 +0100 | [diff] [blame] | 330 | - **Step 1:** Clone the CUE source code from cue.googlesource.com |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 331 | and make sure it's stable by compiling and testing it once: |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 332 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 333 | $ git clone https://cue.googlesource.com/core |
| 334 | $ cd core |
| 335 | $ go test ./... |
| 336 | $ go install ./cmd/cue |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 337 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 338 | |
Marcel van Lohuizen | 3c3de28 | 2018-12-20 21:31:08 +0100 | [diff] [blame] | 339 | - **Step 2:** Prepare changes in a new branch, created from the master branch. |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 340 | To commit the changes, use `git` `codereview` `change`; that |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 341 | will create or amend a single commit in the branch. |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 342 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 343 | $ git checkout -b mybranch |
| 344 | $ [edit files...] |
| 345 | $ git add [files...] |
| 346 | $ git codereview change # create commit in the branch |
| 347 | $ [edit again...] |
| 348 | $ git add [files...] |
| 349 | $ git codereview change # amend the existing commit with new changes |
| 350 | $ [etc.] |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 351 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 352 | |
Marcel van Lohuizen | 3c3de28 | 2018-12-20 21:31:08 +0100 | [diff] [blame] | 353 | - **Step 3:** Test your changes, re-running `go test`. |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 354 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 355 | $ go test ./... # recompile and test |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 356 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 357 | |
Marcel van Lohuizen | 3c3de28 | 2018-12-20 21:31:08 +0100 | [diff] [blame] | 358 | - **Step 4:** Send the changes for review to Gerrit using `git` |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 359 | `codereview` `mail` (which doesn't use e-mail, despite the name). |
| 360 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 361 | $ git codereview mail # send changes to Gerrit |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 362 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 363 | |
Marcel van Lohuizen | 3c3de28 | 2018-12-20 21:31:08 +0100 | [diff] [blame] | 364 | - **Step 5:** After a review, apply changes to the same single commit |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 365 | and mail them to Gerrit again: |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 366 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 367 | $ [edit files...] |
| 368 | $ git add [files...] |
| 369 | $ git codereview change # update same commit |
| 370 | $ git codereview mail # send to Gerrit again |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 371 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 372 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 373 | The rest of this section describes these steps in more detail. |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 374 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 375 | |
| 376 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 377 | ### Step 1: Clone the CUE source code |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 378 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 379 | In addition to a recent CUE installation, you need to have a local copy of the source |
| 380 | checked out from the correct repository. |
| 381 | You can check out the CUE source repo onto your local file system anywhere |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 382 | you want as long as it's outside your `GOPATH`. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 383 | Either clone from |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 384 | `cue.googlesource.com` or from GitHub: |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 385 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 386 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 387 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 388 | $ git clone https://github.com/cuelang/core # or https://cue.googlesource.com/core |
| 389 | $ cd core |
| 390 | $ go test ./... |
| 391 | # go install ./cmd/cue |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 392 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 393 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 394 | ### Step 2: Prepare changes in a new branch |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 395 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 396 | Each CUE change must be made in a separate branch, created from the master branch. |
| 397 | You can use |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 398 | the normal `git` commands to create a branch and add changes to the |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 399 | staging area: |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 400 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 401 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 402 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 403 | $ git checkout -b mybranch |
| 404 | $ [edit files...] |
| 405 | $ git add [files...] |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 406 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 407 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 408 | To commit changes, instead of `git commit`, use `git codereview change`. |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 409 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 410 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 411 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 412 | $ git codereview change |
| 413 | (open $EDITOR) |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 414 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 415 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 416 | You can edit the commit description in your favorite editor as usual. |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 417 | The `git` `codereview` `change` command |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 418 | will automatically add a unique Change-Id line near the bottom. |
| 419 | That line is used by Gerrit to match successive uploads of the same change. |
| 420 | Do not edit or delete it. |
| 421 | A Change-Id looks like this: |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 422 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 423 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 424 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 425 | Change-Id: I2fbdbffb3aab626c4b6f56348861b7909e3e8990 |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 426 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 427 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 428 | The tool also checks that you've |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 429 | run `go` `fmt` over the source code, and that |
| 430 | the commit message follows the suggested format. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 431 | |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 432 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 433 | If you need to edit the files again, you can stage the new changes and |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 434 | re-run `git` `codereview` `change`: each subsequent |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 435 | run will amend the existing commit while preserving the Change-Id. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 436 | |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 437 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 438 | Make sure that you always keep a single commit in each branch. |
| 439 | If you add more |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 440 | commits by mistake, you can use `git` `rebase` to |
| 441 | [squash them together](https://stackoverflow.com/questions/31668794/squash-all-your-commits-in-one-before-a-pull-request-in-github) |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 442 | into a single one. |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 443 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 444 | |
| 445 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 446 | ### Step 3: Test your changes |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 447 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 448 | You've written and tested your code, but |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 449 | before sending code out for review, run <i>all the tests for the whole |
| 450 | tree</i> to make sure the changes don't break other packages or programs: |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 451 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 452 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 453 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 454 | $ go test ./... |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 455 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 456 | |
| 457 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 458 | ### Step 4: Send changes for review |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 459 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 460 | Once the change is ready and tested over the whole tree, send it for review. |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 461 | This is done with the `mail` sub-command which, despite its name, doesn't |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 462 | directly mail anything; it just sends the change to Gerrit: |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 463 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 464 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 465 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 466 | $ git codereview mail |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 467 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 468 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 469 | Gerrit assigns your change a number and URL, which `git` `codereview` `mail` will print, something like: |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 470 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 471 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 472 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 473 | remote: New Changes: |
| 474 | remote: https://cue-review.googlesource.com/99999 math: improved Sin, Cos and Tan precision for very large arguments |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 475 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 476 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 477 | If you get an error instead, check the |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 478 | Troubleshooting mail errors section. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 479 | |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 480 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 481 | If your change relates to an open GitHub issue and you have followed the |
| 482 | suggested commit message format, the issue will be updated in a few minutes by a bot, |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 483 | linking your Gerrit change to it in the comments. |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 484 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 485 | |
| 486 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 487 | ### Step 5: Revise changes after a review |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 488 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 489 | CUE maintainers will review your code on Gerrit, and you will get notifications via e-mail. |
| 490 | You can see the review on Gerrit and comment on them there. |
| 491 | You can also reply |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 492 | [using e-mail](https://gerrit-review.googlesource.com/Documentation/intro-user.html#reply-by-email) |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 493 | if you prefer. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 494 | |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 495 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 496 | If you need to revise your change after the review, edit the files in |
| 497 | the same branch you previously created, add them to the Git staging |
| 498 | area, and then amend the commit with |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 499 | `git` `codereview` `change`: |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 500 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 501 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 502 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 503 | $ git codereview change # amend current commit |
| 504 | (open $EDITOR) |
| 505 | $ git codereview mail # send new changes to Gerrit |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 506 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 507 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 508 | If you don't need to change the commit description, just save and exit from the editor. |
| 509 | Remember not to touch the special Change-Id line. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 510 | |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 511 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 512 | Again, make sure that you always keep a single commit in each branch. |
| 513 | If you add more |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 514 | commits by mistake, you can use `git rebase` to |
| 515 | [squash them together](https://stackoverflow.com/questions/31668794/squash-all-your-commits-in-one-before-a-pull-request-in-github) |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 516 | into a single one. |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 517 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 518 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 519 | ## Good commit messages |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 520 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 521 | Commit messages in CUE follow a specific set of conventions, |
| 522 | which we discuss in this section. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 523 | |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 524 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 525 | Here is an example of a good one: |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 526 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 527 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 528 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 529 | math: improve Sin, Cos and Tan precision for very large arguments |
| 530 | |
| 531 | The existing implementation has poor numerical properties for |
| 532 | large arguments, so use the McGillicutty algorithm to improve |
| 533 | accuracy above 1e10. |
| 534 | |
| 535 | The algorithm is described at https://wikipedia.org/wiki/McGillicutty_Algorithm |
| 536 | |
| 537 | Fixes #159 |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 538 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 539 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 540 | ### First line |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 541 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 542 | The first line of the change description is conventionally a short one-line |
| 543 | summary of the change, prefixed by the primary affected package. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 544 | |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 545 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 546 | A rule of thumb is that it should be written so to complete the sentence |
| 547 | "This change modifies CUE to _____." |
| 548 | That means it does not start with a capital letter, is not a complete sentence, |
| 549 | and actually summarizes the result of the change. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 550 | |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 551 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 552 | Follow the first line by a blank line. |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 553 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 554 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 555 | ### Main content |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 556 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 557 | The rest of the description elaborates and should provide context for the |
| 558 | change and explain what it does. |
| 559 | Write in complete sentences with correct punctuation, just like |
| 560 | for your comments in CUE. |
| 561 | Don't use HTML, Markdown, or any other markup language. |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 562 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 563 | |
| 564 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 565 | ### Referencing issues |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 566 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 567 | The special notation "Fixes #12345" associates the change with issue 12345 in the |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 568 | [CUE issue tracker](https://cuelang.org/issue/12345) |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 569 | When this change is eventually applied, the issue |
| 570 | tracker will automatically mark the issue as fixed. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 571 | |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 572 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 573 | If the change is a partial step towards the resolution of the issue, |
| 574 | uses the notation "Updates #12345". |
| 575 | This will leave a comment in the issue |
| 576 | linking back to the change in Gerrit, but it will not close the issue |
| 577 | when the change is applied. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 578 | |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 579 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 580 | If you are sending a change against a subrepository, you must use |
| 581 | the fully-qualified syntax supported by GitHub to make sure the change is |
| 582 | linked to the issue in the main repository, not the subrepository. |
| 583 | All issues are tracked in the main repository's issue tracker. |
| 584 | The correct form is "Fixes cuelang/core#159". |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 585 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 586 | |
| 587 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 588 | ## The review process |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 589 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 590 | This section explains the review process in detail and how to approach |
| 591 | reviews after a change has been mailed. |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 592 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 593 | |
| 594 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 595 | ### Common beginner mistakes |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 596 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 597 | When a change is sent to Gerrit, it is usually triaged within a few days. |
| 598 | A maintainer will have a look and provide some initial review that for first-time |
| 599 | contributors usually focuses on basic cosmetics and common mistakes. |
| 600 | These include things like: |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 601 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 602 | |
Marcel van Lohuizen | 3c3de28 | 2018-12-20 21:31:08 +0100 | [diff] [blame] | 603 | - Commit message not following the suggested |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 604 | format. |
Marcel van Lohuizen | 3c3de28 | 2018-12-20 21:31:08 +0100 | [diff] [blame] | 605 | - The lack of a linked GitHub issue. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 606 | The vast majority of changes |
| 607 | require a linked issue that describes the bug or the feature that the change |
| 608 | fixes or implements, and consensus should have been reached on the tracker |
| 609 | before proceeding with it. |
| 610 | Gerrit reviews do not discuss the merit of the change, |
| 611 | just its implementation. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 612 | Only trivial or cosmetic changes will be accepted without an associated issue. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 613 | |
Marcel van Lohuizen | 0949135 | 2018-12-20 20:20:54 +0100 | [diff] [blame] | 614 | <!-- TODO |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 615 | <li> |
| 616 | Change sent during the freeze phase of the development cycle, when the tree |
| 617 | is closed for general changes. |
| 618 | In this case, |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 619 | a maintainer might review the code with a line such as `R=cue1.1`, |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 620 | which means that it will be reviewed later when the tree opens for a new |
| 621 | development window. |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 622 | You can add `R=cue1.XX` as a comment yourself |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 623 | if you know that it's not the correct time frame for the change. |
| 624 | </li> |
Marcel van Lohuizen | 0949135 | 2018-12-20 20:20:54 +0100 | [diff] [blame] | 625 | --> |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 626 | |
| 627 | <!-- |
| 628 | TODO |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 629 | ### Trybots |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 630 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 631 | After an initial reading of your change, maintainers will trigger trybots, |
| 632 | a cluster of servers that will run the full test suite on several different |
| 633 | architectures. |
| 634 | Most trybots complete in a few minutes, at which point a link will |
| 635 | be posted in Gerrit where you can see the results. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 636 | |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 637 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 638 | If the trybot run fails, follow the link and check the full logs of the |
| 639 | platforms on which the tests failed. |
| 640 | Try to understand what broke, update your patch to fix it, and upload again. |
| 641 | Maintainers will trigger a new trybot run to see |
| 642 | if the problem was fixed. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 643 | |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 644 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 645 | Sometimes, the tree can be broken on some platforms for a few hours; if |
| 646 | the failure reported by the trybot doesn't seem related to your patch, go to the |
| 647 | <a href="https://build.cuelang.org">Build Dashboard</a> and check if the same |
| 648 | failure appears in other recent commits on the same platform. |
| 649 | In this case, |
| 650 | feel free to write a comment in Gerrit to mention that the failure is |
| 651 | unrelated to your change, to help maintainers understand the situation. |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 652 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 653 | --> |
| 654 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 655 | ### Reviews |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 656 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 657 | The CUE community values very thorough reviews. |
| 658 | Think of each review comment like a ticket: you are expected to somehow "close" it |
| 659 | by acting on it, either by implementing the suggestion or convincing the |
| 660 | reviewer otherwise. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 661 | |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 662 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 663 | After you update the change, go through the review comments and make sure |
| 664 | to reply to every one. |
| 665 | You can click the "Done" button to reply |
| 666 | indicating that you've implemented the reviewer's suggestion; otherwise, |
| 667 | click on "Reply" and explain why you have not, or what you have done instead. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 668 | |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 669 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 670 | It is perfectly normal for changes to go through several round of reviews, |
| 671 | with one or more reviewers making new comments every time |
| 672 | and then waiting for an updated change before reviewing again. |
| 673 | This cycle happens even for experienced contributors, so |
| 674 | don't be discouraged by it. |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 675 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 676 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 677 | ### Voting conventions |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 678 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 679 | As they near a decision, reviewers will make a "vote" on your change. |
| 680 | The Gerrit voting system involves an integer in the range -2 to +2: |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 681 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 682 | |
Marcel van Lohuizen | 3c3de28 | 2018-12-20 21:31:08 +0100 | [diff] [blame] | 683 | - **+2** The change is approved for being merged. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 684 | Only CUE maintainers can cast a +2 vote. |
Marcel van Lohuizen | 3c3de28 | 2018-12-20 21:31:08 +0100 | [diff] [blame] | 685 | - **+1** The change looks good, but either the reviewer is requesting |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 686 | minor changes before approving it, or they are not a maintainer and cannot |
| 687 | approve it, but would like to encourage an approval. |
Marcel van Lohuizen | 3c3de28 | 2018-12-20 21:31:08 +0100 | [diff] [blame] | 688 | - **-1** The change is not good the way it is but might be fixable. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 689 | A -1 vote will always have a comment explaining why the change is unacceptable. |
Marcel van Lohuizen | 3c3de28 | 2018-12-20 21:31:08 +0100 | [diff] [blame] | 690 | - **-2** The change is blocked by a maintainer and cannot be approved. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 691 | Again, there will be a comment explaining the decision. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 692 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 693 | ### Submitting an approved change |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 694 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 695 | After the code has been +2'ed, an approver will |
| 696 | apply it to the master branch using the Gerrit user interface. |
| 697 | This is called "submitting the change". |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 698 | |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 699 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 700 | The two steps (approving and submitting) are separate because in some cases maintainers |
| 701 | may want to approve it but not to submit it right away (for instance, |
| 702 | the tree could be temporarily frozen). |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 703 | |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 704 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 705 | Submitting a change checks it into the repository. |
| 706 | The change description will include a link to the code review, |
| 707 | which will be updated with a link to the change |
| 708 | in the repository. |
| 709 | Since the method used to integrate the changes is Git's "Cherry Pick", |
| 710 | the commit hashes in the repository will be changed by |
| 711 | the submit operation. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 712 | |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 713 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 714 | If your change has been approved for a few days without being |
| 715 | submitted, feel free to write a comment in Gerrit requesting |
| 716 | submission. |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 717 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 718 | |
| 719 | |
| 720 | <!-- |
| 721 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 722 | ### More information |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 723 | |
| 724 | TODO |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 725 | In addition to the information here, the CUE community maintains a <a |
| 726 | href="https://cuelang.org/wiki/CodeReview">CodeReview</a> wiki page. |
| 727 | Feel free to contribute to this page as you learn more about the review process. |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 728 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 729 | --> |
| 730 | |
| 731 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 732 | ## Miscellaneous topics |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 733 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 734 | This section collects a number of other comments that are |
| 735 | outside the issue/edit/code review/submit process itself. |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 736 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 737 | |
| 738 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 739 | ### Copyright headers |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 740 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 741 | Files in the CUE repository don't list author names, both to avoid clutter |
| 742 | and to avoid having to keep the lists up to date. |
| 743 | Instead, your name will appear in the |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 744 | [change log](https://cue.googlesource.com/cue/+log) and in the |
| 745 | [`CONTRIBUTORS`](../CONTRIBUTORS) file and perhaps the |
| 746 | [`AUTHORS`](../AUTHORS) file. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 747 | These files are automatically generated from the commit logs periodically. |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 748 | The [`AUTHORS`](../AUTHORS) file defines who “The CUE |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 749 | Authors”—the copyright holders—are. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 750 | |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 751 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 752 | New files that you contribute should use the standard copyright header: |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 753 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 754 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 755 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 756 | // Copyright 2018 The CUE Authors |
| 757 | // |
| 758 | // Licensed under the Apache License, Version 2.0 (the "License"); |
| 759 | // you may not use this file except in compliance with the License. |
| 760 | // You may obtain a copy of the License at |
| 761 | // |
| 762 | // http://www.apache.org/licenses/LICENSE-2.0 |
| 763 | // |
| 764 | // Unless required by applicable law or agreed to in writing, software |
| 765 | // distributed under the License is distributed on an "AS IS" BASIS, |
| 766 | // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. |
| 767 | // See the License for the specific language governing permissions and |
| 768 | // limitations under the License. |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 769 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 770 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 771 | (Use the current year if you're reading this in 2019 or beyond.) |
| 772 | Files in the repository are copyrighted the year they are added. |
| 773 | Do not update the copyright year on files that you change. |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 774 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 775 | |
| 776 | |
| 777 | |
| 778 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 779 | ### Troubleshooting mail errors |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 780 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 781 | The most common way that the `git` `codereview` `mail` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 782 | command fails is because the e-mail address in the commit does not match the one |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 783 | that you used during the registration process. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 784 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 785 | If you see something like... |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 786 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 787 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 788 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 789 | remote: Processing changes: refs: 1, done |
| 790 | remote: |
| 791 | remote: ERROR: In commit ab13517fa29487dcf8b0d48916c51639426c5ee9 |
| 792 | remote: ERROR: author email address XXXXXXXXXXXXXXXXXXX |
| 793 | remote: ERROR: does not match your user account. |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 794 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 795 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 796 | you need to configure Git for this repository to use the |
| 797 | e-mail address that you registered with. |
| 798 | To change the e-mail address to ensure this doesn't happen again, run: |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 799 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 800 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 801 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 802 | $ git config user.email email@address.com |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 803 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 804 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 805 | Then change the commit to use this alternative e-mail address with this command: |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 806 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 807 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 808 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 809 | $ git commit --amend --author="Author Name <email@address.com>" |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 810 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 811 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 812 | Then retry by running: |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 813 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 814 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 815 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 816 | $ git codereview mail |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 817 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 818 | |
| 819 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 820 | ### Quickly testing your changes |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 821 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 822 | Running `go test ./...` for every single change to the code tree |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 823 | is burdensome. |
| 824 | Even though it is strongly suggested to run it before |
| 825 | sending a change, during the normal development cycle you may want |
| 826 | to compile and test only the package you are developing. |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 827 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 828 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 829 | In this section, we'll call the directory into which you cloned the CUE repository `$CUEDIR`. |
| 830 | As CUE uses Go modules, The `cue` tool built by |
| 831 | `go install` will be installed in the `bin/go` in your |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 832 | home directory by default. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 833 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 834 | If you're changing the CUE APIs or code, you can test the results in just |
| 835 | this package directory. |
| 836 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 837 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 838 | $ cd $CUEDIR/cue |
| 839 | $ [make changes...] |
| 840 | $ go test |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 841 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 842 | |
| 843 | You don't need to build a new cue tool to test it. |
| 844 | Instead you can run the tests from the root. |
| 845 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 846 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 847 | $ cd $CUEDIR |
| 848 | $ go test ./... |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 849 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 850 | |
| 851 | To use the new tool you would still need to build and install it. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 852 | |
| 853 | |
| 854 | <!-- |
| 855 | TODO |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 856 | ### Contributing to subrepositories (cuelang.org/x/...) |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 857 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 858 | If you are contributing a change to a subrepository, obtain the |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 859 | CUE package using `go get`. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 860 | For example, to contribute |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 861 | to `cuelang.org/x/editor/vscode`, check out the code by running: |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 862 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 863 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 864 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 865 | $ go get -d cuelang.org/editor/vscode |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 866 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 867 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 868 | Then, change your directory to the package's source directory |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 869 | (`$GOPATH/src/cuelang.org/x/oauth2`), and follow the |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 870 | normal contribution flow. |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 871 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 872 | --> |
| 873 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 874 | ### Specifying a reviewer / CCing others |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 875 | |
| 876 | <!-- |
| 877 | TODO: |
| 878 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 879 | Unless explicitly told otherwise, such as in the discussion leading |
| 880 | up to sending in the change, it's better not to specify a reviewer. |
| 881 | All changes are automatically CC'ed to the |
| 882 | <a href="https://groups.google.com/group/cue-codereviews">cue-codereviews@googlegroups.com</a> |
| 883 | mailing list. |
| 884 | If this is your first ever change, there may be a moderation |
| 885 | delay before it appears on the mailing list, to prevent spam. |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 886 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 887 | --> |
| 888 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 889 | You can specify a reviewer or CC interested parties |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 890 | using the `-r` or `-cc` options. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 891 | Both accept a comma-separated list of e-mail addresses: |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 892 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 893 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 894 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 895 | $ git codereview mail -r joe@cuelang.org -cc mabel@example.com,math-nuts@swtch.com |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 896 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 897 | |
| 898 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 899 | ### Synchronize your client |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 900 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 901 | While you were working, others might have submitted changes to the repository. |
| 902 | To update your local branch, run |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 903 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 904 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 905 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 906 | $ git codereview sync |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 907 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 908 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 909 | (Under the covers this runs |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 910 | `git` `pull` `-r`.) |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 911 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 912 | |
| 913 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 914 | ### Reviewing code by others |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 915 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 916 | As part of the review process reviewers can propose changes directly (in the |
| 917 | GitHub workflow this would be someone else attaching commits to a pull request). |
| 918 | |
| 919 | You can import these changes proposed by someone else into your local Git repository. |
| 920 | On the Gerrit review page, click the "Download ▼" link in the upper right |
| 921 | corner, copy the "Checkout" command and run it from your local Git repo. |
| 922 | It will look something like this: |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 923 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 924 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 925 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 926 | $ git fetch https://cue.googlesource.com/review refs/changes/21/13245/1 && git checkout FETCH_HEAD |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 927 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 928 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 929 | To revert, change back to the branch you were working in. |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 930 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 931 | |
Marcel van Lohuizen | 3c3de28 | 2018-12-20 21:31:08 +0100 | [diff] [blame] | 932 | ### Set up git aliases |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 933 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 934 | The `git-codereview` command can be run directly from the shell |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 935 | by typing, for instance, |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 936 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 937 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 938 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 939 | $ git codereview sync |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 940 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 941 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 942 | but it is more convenient to set up aliases for `git-codereview`'s own |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 943 | subcommands, so that the above becomes, |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 944 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 945 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 946 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 947 | $ git sync |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 948 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 949 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 950 | The `git-codereview` subcommands have been chosen to be distinct from |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 951 | Git's own, so it's safe to define these aliases. |
| 952 | To install them, copy this text into your |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 953 | Git configuration file (usually `.gitconfig` in your home directory): |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 954 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 955 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 956 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 957 | [alias] |
| 958 | change = codereview change |
| 959 | gofmt = codereview gofmt |
| 960 | mail = codereview mail |
| 961 | pending = codereview pending |
| 962 | submit = codereview submit |
| 963 | sync = codereview sync |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 964 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 965 | |
| 966 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 967 | ### Sending multiple dependent changes |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 968 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 969 | Advanced users may want to stack up related commits in a single branch. |
| 970 | Gerrit allows for changes to be dependent on each other, forming such a dependency chain. |
| 971 | Each change will need to be approved and submitted separately but the dependency |
| 972 | will be visible to reviewers. |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 973 | |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 974 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 975 | To send out a group of dependent changes, keep each change as a different commit under |
| 976 | the same branch, and then run: |
Marcel van Lohuizen | cd59e42 | 2018-12-20 20:56:05 +0100 | [diff] [blame] | 977 | |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 978 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 979 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 980 | $ git codereview mail HEAD |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 981 | ``` |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 982 | |
Marcel van Lohuizen | 5727103 | 2018-12-20 21:23:10 +0100 | [diff] [blame] | 983 | Make sure to explicitly specify `HEAD`, which is usually not required when sending |
Marcel van Lohuizen | 969a783 | 2018-11-22 19:43:51 +0100 | [diff] [blame] | 984 | single changes. |
Marcel van Lohuizen | 0949135 | 2018-12-20 20:20:54 +0100 | [diff] [blame] | 985 | |