Andrew Gerrand | 7cb21a7 | 2012-01-19 11:24:54 +1100 | [diff] [blame] | 1 | <!--{ |
| 2 | "Title": "Contribution Guidelines" |
| 3 | }--> |
Russ Cox | fb39a4d | 2009-10-23 15:24:08 -0700 | [diff] [blame] | 4 | |
Russ Cox | 6c7f9f6 | 2009-11-09 16:56:17 -0800 | [diff] [blame] | 5 | <h2 id="Introduction">Introduction</h2> |
Rob Pike | 5b72f9c | 2009-11-01 20:48:21 -0800 | [diff] [blame] | 6 | |
| 7 | <p> |
Russ Cox | d55abfd | 2009-12-09 14:05:12 -0800 | [diff] [blame] | 8 | This document explains how to contribute changes to the Go project. |
Russ Cox | 696e802 | 2009-11-07 18:56:00 -0800 | [diff] [blame] | 9 | It assumes you have installed Go using the |
Jonathan Feinberg | 452f40f | 2012-09-26 14:39:31 -0400 | [diff] [blame] | 10 | <a href="/doc/install/source">installation instructions</a> and |
Russ Cox | d55abfd | 2009-12-09 14:05:12 -0800 | [diff] [blame] | 11 | have <a href="code.html">written and tested your code</a>. |
| 12 | (Note that the <code>gccgo</code> frontend lives elsewhere; |
Ian Lance Taylor | 2528f33 | 2009-11-06 14:15:41 -0800 | [diff] [blame] | 13 | see <a href="gccgo_contribute.html">Contributing to gccgo</a>.) |
Russ Cox | 38a41ee | 2009-11-06 10:04:22 -0800 | [diff] [blame] | 14 | </p> |
Russ Cox | fb39a4d | 2009-10-23 15:24:08 -0700 | [diff] [blame] | 15 | |
Rob Pike | 96ee38b | 2009-12-17 12:12:47 +1100 | [diff] [blame] | 16 | <h2 id="Design">Discuss your design</h2> |
| 17 | |
| 18 | <p> |
| 19 | The project welcomes submissions but please let everyone know what |
| 20 | you're working on if you want it to become part of the main repository. |
| 21 | </p> |
| 22 | |
| 23 | <p> |
| 24 | Before undertaking to write something new for the Go project, send |
Brad Fitzpatrick | 967901a | 2014-03-05 14:09:03 -0800 | [diff] [blame] | 25 | mail to the <a href="https://groups.google.com/group/golang-nuts">mailing |
Rob Pike | 96ee38b | 2009-12-17 12:12:47 +1100 | [diff] [blame] | 26 | list</a> to discuss what you plan to do. This gives everyone a |
| 27 | chance to validate the design, helps prevent duplication of effort, |
| 28 | and ensures that the idea fits inside the goals for the language |
| 29 | and tools. It also guarantees that the design is sound before code |
| 30 | is written; the code review tool is not the place for high-level |
| 31 | discussions. |
| 32 | </p> |
| 33 | |
| 34 | <p> |
| 35 | In short, send mail before you code. |
| 36 | And don't start the discussion by mailing a change list! |
| 37 | </p> |
| 38 | |
Russ Cox | d55abfd | 2009-12-09 14:05:12 -0800 | [diff] [blame] | 39 | <h2 id="Testing">Testing redux</h2> |
Russ Cox | 830813f | 2009-11-08 21:08:27 -0800 | [diff] [blame] | 40 | |
| 41 | <p> |
Russ Cox | d55abfd | 2009-12-09 14:05:12 -0800 | [diff] [blame] | 42 | You've <a href="code.html">written and tested your code</a>, but |
| 43 | before sending code out for review, run all the tests for the whole |
| 44 | tree to make sure the changes don't break other packages or programs: |
Russ Cox | 38a41ee | 2009-11-06 10:04:22 -0800 | [diff] [blame] | 45 | </p> |
| 46 | |
| 47 | <pre> |
Nathan John Youngman | d5f208c | 2014-03-17 09:35:04 +1100 | [diff] [blame] | 48 | $ cd go/src |
| 49 | $ ./all.bash |
Russ Cox | 38a41ee | 2009-11-06 10:04:22 -0800 | [diff] [blame] | 50 | </pre> |
| 51 | |
| 52 | <p> |
Nathan John Youngman | d5f208c | 2014-03-17 09:35:04 +1100 | [diff] [blame] | 53 | (To build under Windows use <code>all.bat</code>.) |
| 54 | </p> |
| 55 | |
| 56 | <p> |
Shenghou Ma | 4c473c0 | 2012-10-11 23:33:57 +0800 | [diff] [blame] | 57 | After running for a while, the command should print "<code>ALL TESTS PASSED</code>". |
Russ Cox | 38a41ee | 2009-11-06 10:04:22 -0800 | [diff] [blame] | 58 | </p> |
| 59 | |
Russ Cox | 6c7f9f6 | 2009-11-09 16:56:17 -0800 | [diff] [blame] | 60 | <h2 id="Code_review">Code review</h2> |
Russ Cox | 38a41ee | 2009-11-06 10:04:22 -0800 | [diff] [blame] | 61 | |
| 62 | <p> |
| 63 | Changes to Go must be reviewed before they are submitted, |
| 64 | no matter who makes the change. |
Russ Cox | 9ad14c9 | 2009-11-06 10:33:46 -0800 | [diff] [blame] | 65 | (In exceptional cases, such as fixing a build, the review can |
| 66 | follow shortly after submitting.) |
Russ Cox | 38a41ee | 2009-11-06 10:04:22 -0800 | [diff] [blame] | 67 | A Mercurial extension helps manage the code review process. |
| 68 | The extension is included in the Go source tree but needs |
| 69 | to be added to your Mercurial configuration. |
| 70 | </p> |
| 71 | |
Russ Cox | 9ad14c9 | 2009-11-06 10:33:46 -0800 | [diff] [blame] | 72 | <h3>Caveat for Mercurial aficionados</h3> |
| 73 | |
Russ Cox | 38a41ee | 2009-11-06 10:04:22 -0800 | [diff] [blame] | 74 | <p> |
| 75 | <i>Using Mercurial with the code review extension is not the same |
Russ Cox | 9ad14c9 | 2009-11-06 10:33:46 -0800 | [diff] [blame] | 76 | as using standard Mercurial.</i> |
Russ Cox | 38a41ee | 2009-11-06 10:04:22 -0800 | [diff] [blame] | 77 | </p> |
| 78 | |
| 79 | <p> |
Russ Cox | 9ad14c9 | 2009-11-06 10:33:46 -0800 | [diff] [blame] | 80 | The Go repository is maintained as a single line of reviewed changes; |
| 81 | we prefer to avoid the complexity of Mercurial's arbitrary change graph. |
| 82 | The code review extension helps here: its <code>hg submit</code> command |
| 83 | automatically checks for and warns about the local repository |
| 84 | being out of date compared to the remote one. |
| 85 | The <code>hg submit</code> command also verifies other |
| 86 | properties about the Go repository. |
Russ Cox | 696e802 | 2009-11-07 18:56:00 -0800 | [diff] [blame] | 87 | For example, |
Russ Cox | 9ad14c9 | 2009-11-06 10:33:46 -0800 | [diff] [blame] | 88 | it checks that Go code being checked in is formatted in the standard style, |
| 89 | as defined by <a href="/cmd/gofmt">gofmt</a>, |
| 90 | and it checks that the author of the code is properly recorded for |
| 91 | <a href="#copyright">copyright purposes</a>. |
| 92 | </p> |
| 93 | |
| 94 | <p> |
| 95 | To help ensure changes are only created by <code>hg submit</code>, |
| 96 | the code review extension disables the standard <code>hg commit</code> |
| 97 | command. |
| 98 | </p> |
| 99 | |
Russ Cox | 38a41ee | 2009-11-06 10:04:22 -0800 | [diff] [blame] | 100 | <h3>Configure the extension</h3> |
| 101 | |
Nathan John Youngman | d5f208c | 2014-03-17 09:35:04 +1100 | [diff] [blame] | 102 | <p>Edit <code>.hg/hgrc</code> in the root of your Go checkout to add:</p> |
Russ Cox | 38a41ee | 2009-11-06 10:04:22 -0800 | [diff] [blame] | 103 | |
| 104 | <pre> |
| 105 | [extensions] |
Nathan John Youngman | d5f208c | 2014-03-17 09:35:04 +1100 | [diff] [blame] | 106 | codereview = /path/to/go/lib/codereview/codereview.py |
Russ Cox | d55abfd | 2009-12-09 14:05:12 -0800 | [diff] [blame] | 107 | |
| 108 | [ui] |
| 109 | username = Your Name <you@server.dom> |
Russ Cox | 38a41ee | 2009-11-06 10:04:22 -0800 | [diff] [blame] | 110 | </pre> |
| 111 | |
David Symonds | 156d85c | 2012-10-08 22:24:41 +1100 | [diff] [blame] | 112 | <p> |
Russ Cox | d55abfd | 2009-12-09 14:05:12 -0800 | [diff] [blame] | 113 | The <code>username</code> information will not be used unless |
| 114 | you are a committer (see below), but Mercurial complains if it is missing. |
Russ Cox | 38a41ee | 2009-11-06 10:04:22 -0800 | [diff] [blame] | 115 | </p> |
| 116 | |
Nathan John Youngman | d5f208c | 2014-03-17 09:35:04 +1100 | [diff] [blame] | 117 | <p> |
| 118 | As the codereview extension is only enabled for your Go checkout, the remainder of this document assumes you |
| 119 | are inside the go directory when issuing commands. |
| 120 | </p> |
| 121 | |
| 122 | <p>To contribute to subrepositories, edit the <code>.hg/hgrc</code> for each |
| 123 | subrepository in the same way. For example, add the codereview extension to |
| 124 | <code>code.google.com/p/go.tools/.hg/hgrc</code>. |
| 125 | </p> |
| 126 | |
Patrick Higgins | 6bf6cae | 2013-06-05 21:09:43 -0700 | [diff] [blame] | 127 | <h3>Understanding the extension</h3> |
| 128 | |
| 129 | <p>After adding the code review extension, you can run</p> |
| 130 | |
| 131 | <pre> |
| 132 | $ hg help codereview |
| 133 | </pre> |
| 134 | |
| 135 | <p>to learn more about its commands. To learn about a specific code-review-specific |
| 136 | command such as <code>change</code>, run</p> |
| 137 | |
| 138 | <pre> |
| 139 | $ hg help change |
| 140 | </pre> |
| 141 | |
Russ Cox | 111fcf1 | 2012-12-11 13:36:43 -0500 | [diff] [blame] | 142 | <p> |
Nathan John Youngman | d5f208c | 2014-03-17 09:35:04 +1100 | [diff] [blame] | 143 | Windows users may need to perform extra steps to get the code review |
Rick Arnold | cea78cb | 2013-03-11 12:14:42 +1100 | [diff] [blame] | 144 | extension working. See the |
Nathan John Youngman | d5f208c | 2014-03-17 09:35:04 +1100 | [diff] [blame] | 145 | <a href="https://code.google.com/p/go-wiki/wiki/CodeReview">CodeReview page</a> |
Brad Fitzpatrick | 967901a | 2014-03-05 14:09:03 -0800 | [diff] [blame] | 146 | on the <a href="https://code.google.com/p/go-wiki/wiki">Go Wiki</a> for details. |
Rick Arnold | cea78cb | 2013-03-11 12:14:42 +1100 | [diff] [blame] | 147 | </p> |
| 148 | |
Russ Cox | 38a41ee | 2009-11-06 10:04:22 -0800 | [diff] [blame] | 149 | <h3>Log in to the code review site.</h3> |
| 150 | |
Russ Cox | 38a41ee | 2009-11-06 10:04:22 -0800 | [diff] [blame] | 151 | <p> |
| 152 | The code review server uses a Google Account to authenticate. |
| 153 | (If you can use the account to |
Russ Cox | 830813f | 2009-11-08 21:08:27 -0800 | [diff] [blame] | 154 | <a href="https://www.google.com/accounts/Login?hl=en&continue=http://www.google.com/">sign in at google.com</a>, |
Caleb Spare | 41f32e0 | 2013-01-06 22:44:16 -0500 | [diff] [blame] | 155 | you can use it to sign in to the code review server.) |
Russ Cox | d55abfd | 2009-12-09 14:05:12 -0800 | [diff] [blame] | 156 | The email address you use on the Code Review site |
Brad Fitzpatrick | 967901a | 2014-03-05 14:09:03 -0800 | [diff] [blame] | 157 | will be recorded in the <a href="https://code.google.com/p/go/source/list">Mercurial change log</a> |
Russ Cox | d55abfd | 2009-12-09 14:05:12 -0800 | [diff] [blame] | 158 | and in the <a href="/CONTRIBUTORS"><code>CONTRIBUTORS</code></a> file. |
| 159 | You can <a href="https://www.google.com/accounts/NewAccount">create a Google Account</a> |
| 160 | associated with any address where you receive email. |
Shenghou Ma | 4c473c0 | 2012-10-11 23:33:57 +0800 | [diff] [blame] | 161 | If you've enabled the two-step verification feature, don't forget to generate an |
| 162 | application-specific password and use that when prompted for a password. |
Russ Cox | 38a41ee | 2009-11-06 10:04:22 -0800 | [diff] [blame] | 163 | </p> |
| 164 | |
| 165 | <pre> |
Vish Subramanian | 8910e42 | 2009-11-06 17:08:47 -0800 | [diff] [blame] | 166 | $ hg code-login |
Russ Cox | 38a41ee | 2009-11-06 10:04:22 -0800 | [diff] [blame] | 167 | Email (login for uploading to codereview.appspot.com): rsc@golang.org |
| 168 | Password for rsc@golang.org: |
| 169 | |
| 170 | Saving authentication cookies to /Users/rsc/.codereview_upload_cookies_codereview.appspot.com |
| 171 | </pre> |
| 172 | |
| 173 | <h3>Configure your account settings.</h3> |
| 174 | |
Brad Fitzpatrick | 967901a | 2014-03-05 14:09:03 -0800 | [diff] [blame] | 175 | <p>Edit your <a href="https://codereview.appspot.com/settings">code review settings</a>. |
Russ Cox | 38a41ee | 2009-11-06 10:04:22 -0800 | [diff] [blame] | 176 | Grab a nickname. |
Russ Cox | 696e802 | 2009-11-07 18:56:00 -0800 | [diff] [blame] | 177 | Many people prefer to set the Context option to |
Russ Cox | 38a41ee | 2009-11-06 10:04:22 -0800 | [diff] [blame] | 178 | “Whole file” to see more context when reviewing changes. |
| 179 | </p> |
| 180 | |
| 181 | <p>Once you have chosen a nickname in the settings page, others |
| 182 | can use that nickname as a shorthand for naming reviewers and the CC list. |
| 183 | For example, <code>rsc</code> is an alias for <code>rsc@golang.org</code>. |
| 184 | </p> |
| 185 | |
Andrew Gerrand | 3238705 | 2012-06-05 00:55:45 +1000 | [diff] [blame] | 186 | <h3>Switch to the default branch</h3> |
| 187 | |
| 188 | <p> |
| 189 | Most Go installations use a release branch, but new changes should |
| 190 | only be made to the default branch. (They may be applied later to a release |
| 191 | branch as part of the release process.) |
| 192 | Before making a change, make sure you use the default branch: |
| 193 | </p> |
| 194 | |
| 195 | <pre> |
| 196 | $ hg update default |
| 197 | </pre> |
| 198 | |
Russ Cox | 9ad14c9 | 2009-11-06 10:33:46 -0800 | [diff] [blame] | 199 | <h3>Make a change</h3> |
Russ Cox | 38a41ee | 2009-11-06 10:04:22 -0800 | [diff] [blame] | 200 | |
| 201 | <p> |
| 202 | The entire checked-out tree is writable. |
| 203 | If you need to edit files, just edit them: Mercurial will figure out which ones changed. |
| 204 | You do need to inform Mercurial of added, removed, copied, or renamed files, |
Russ Cox | 696e802 | 2009-11-07 18:56:00 -0800 | [diff] [blame] | 205 | by running |
Russ Cox | 38a41ee | 2009-11-06 10:04:22 -0800 | [diff] [blame] | 206 | <code>hg add</code>, |
| 207 | <code>hg rm</code>, |
| 208 | <code>hg cp</code>, |
| 209 | or |
| 210 | <code>hg mv</code>. |
| 211 | </p> |
| 212 | |
Russ Cox | 38a41ee | 2009-11-06 10:04:22 -0800 | [diff] [blame] | 213 | <p>When you are ready to send a change out for review, run</p> |
| 214 | |
| 215 | <pre> |
| 216 | $ hg change |
| 217 | </pre> |
| 218 | |
| 219 | <p>from any directory in your Go repository. |
| 220 | Mercurial will open a change description file in your editor. |
| 221 | (It uses the editor named by the <code>$EDITOR</code> environment variable, <code>vi</code> by default.) |
| 222 | The file will look like: |
| 223 | </p> |
| 224 | |
| 225 | <pre> |
| 226 | # Change list. |
| 227 | # Lines beginning with # are ignored. |
| 228 | # Multi-line values should be indented. |
| 229 | |
Russ Cox | 696e802 | 2009-11-07 18:56:00 -0800 | [diff] [blame] | 230 | Reviewer: |
| 231 | CC: |
Russ Cox | 38a41ee | 2009-11-06 10:04:22 -0800 | [diff] [blame] | 232 | |
| 233 | Description: |
| 234 | <enter description here> |
| 235 | |
| 236 | Files: |
| 237 | src/pkg/math/sin.go |
| 238 | src/pkg/math/tan.go |
| 239 | src/pkg/regexp/regexp.go |
| 240 | </pre> |
| 241 | |
| 242 | <p> |
| 243 | The <code>Reviewer</code> line lists the reviewers assigned |
| 244 | to this change, and the <code>CC</code> line lists people to |
| 245 | notify about the change. |
| 246 | These can be code review nicknames or arbitrary email addresses. |
Rob Pike | 9ada841 | 2011-05-13 16:25:31 -0700 | [diff] [blame] | 247 | Unless explicitly told otherwise, such as in the discussion leading |
Florian Weimer | b1175be | 2011-12-13 17:45:01 -0500 | [diff] [blame] | 248 | up to sending in the change list, leave the reviewer field blank. |
| 249 | This means that the |
Brad Fitzpatrick | 967901a | 2014-03-05 14:09:03 -0800 | [diff] [blame] | 250 | <a href="https://groups.google.com/group/golang-codereviews">golang-codereviews@googlegroups.com</a> |
Florian Weimer | b1175be | 2011-12-13 17:45:01 -0500 | [diff] [blame] | 251 | mailing list will be used as the reviewer. |
Russ Cox | 38a41ee | 2009-11-06 10:04:22 -0800 | [diff] [blame] | 252 | </p> |
| 253 | |
| 254 | <p> |
| 255 | Replace “<code><enter description here></code>” |
| 256 | with a description of your change. |
Nigel Tao | 3f38342 | 2011-01-05 13:00:08 +1100 | [diff] [blame] | 257 | The first line of the change description is conventionally a one-line |
| 258 | summary of the change, prefixed by the primary affected package, |
| 259 | and is used as the subject for code review mail; the rest of the |
Russ Cox | 38a41ee | 2009-11-06 10:04:22 -0800 | [diff] [blame] | 260 | description elaborates. |
| 261 | </p> |
| 262 | |
| 263 | <p> |
| 264 | The <code>Files</code> section lists all the modified files |
| 265 | in your client. |
| 266 | It is best to keep unrelated changes in different change lists. |
| 267 | In this example, we can include just the changes to package <code>math</code> |
| 268 | by deleting the line mentioning <code>regexp.go</code>. |
Russ Cox | 696e802 | 2009-11-07 18:56:00 -0800 | [diff] [blame] | 269 | </p> |
| 270 | |
| 271 | <p> |
| 272 | After editing, the template might now read: |
Russ Cox | 38a41ee | 2009-11-06 10:04:22 -0800 | [diff] [blame] | 273 | </p> |
| 274 | |
| 275 | <pre> |
| 276 | # Change list. |
| 277 | # Lines beginning with # are ignored. |
| 278 | # Multi-line values should be indented. |
| 279 | |
Shawn Smith | 2f5f193 | 2013-12-29 11:11:28 -0800 | [diff] [blame] | 280 | Reviewer: golang-codereviews@googlegroups.com |
Russ Cox | 38a41ee | 2009-11-06 10:04:22 -0800 | [diff] [blame] | 281 | CC: math-nuts@swtch.com |
| 282 | |
| 283 | Description: |
Nigel Tao | 3f38342 | 2011-01-05 13:00:08 +1100 | [diff] [blame] | 284 | math: improved Sin, Cos and Tan precision for very large arguments. |
Russ Cox | 696e802 | 2009-11-07 18:56:00 -0800 | [diff] [blame] | 285 | |
Russ Cox | 38a41ee | 2009-11-06 10:04:22 -0800 | [diff] [blame] | 286 | See Bimmler and Shaney, ``Extreme sinusoids,'' J. Math 3(14). |
| 287 | Fixes issue 159. |
| 288 | |
| 289 | Files: |
| 290 | src/pkg/math/sin.go |
| 291 | src/pkg/math/tan.go |
| 292 | </pre> |
| 293 | |
| 294 | <p> |
| 295 | The special sentence “Fixes issue 159.” associates |
Brad Fitzpatrick | 967901a | 2014-03-05 14:09:03 -0800 | [diff] [blame] | 296 | the change with issue 159 in the <a href="https://code.google.com/p/go/issues/list">Go issue tracker</a>. |
Russ Cox | 38a41ee | 2009-11-06 10:04:22 -0800 | [diff] [blame] | 297 | When this change is eventually submitted, the issue |
| 298 | tracker will automatically mark the issue as fixed. |
Matthew Dempsky | 7d40387 | 2013-01-10 14:17:20 +1100 | [diff] [blame] | 299 | (These conventions are described in detail by the |
Brad Fitzpatrick | 967901a | 2014-03-05 14:09:03 -0800 | [diff] [blame] | 300 | <a href="https://code.google.com/p/support/wiki/IssueTracker#Integration_with_version_control">Google Project Hosting Issue Tracker documentation</a>.) |
Russ Cox | 38a41ee | 2009-11-06 10:04:22 -0800 | [diff] [blame] | 301 | </p> |
| 302 | |
| 303 | <p> |
| 304 | Save the file and exit the editor.</p> |
| 305 | |
| 306 | <p> |
| 307 | The code review server assigns your change an issue number and URL, |
| 308 | which <code>hg change</code> will print, something like: |
| 309 | </p> |
| 310 | |
| 311 | <pre> |
Brad Fitzpatrick | 967901a | 2014-03-05 14:09:03 -0800 | [diff] [blame] | 312 | CL created: https://codereview.appspot.com/99999 |
Russ Cox | 38a41ee | 2009-11-06 10:04:22 -0800 | [diff] [blame] | 313 | </pre> |
| 314 | |
Bill Thiede | 822b2cb | 2014-07-03 17:42:23 -0400 | [diff] [blame^] | 315 | <h3>Mail the change for review</h3> |
| 316 | |
| 317 | <p> |
| 318 | Creating or uploading the change uploads a copy of the diff to the code review server, |
| 319 | but it does not notify anyone about it. To do that, you need to run <code>hg mail</code> |
| 320 | (see below). |
| 321 | </p> |
| 322 | |
| 323 | <p>To send out a change for review, run <code>hg mail</code> using the change list number |
| 324 | assigned during <code>hg change</code>:</p> |
| 325 | |
| 326 | <pre> |
| 327 | $ hg mail 99999 |
| 328 | </pre> |
| 329 | |
| 330 | <p>You can add to the <code>Reviewer:</code> and <code>CC:</code> lines |
| 331 | using the <code>-r</code> or <code>--cc</code> options. |
| 332 | In the above example, we could have left the <code>Reviewer</code> and <code>CC</code> |
| 333 | lines blank and then run: |
| 334 | </p> |
| 335 | |
| 336 | <pre> |
| 337 | $ hg mail -r golang-codereviews@googlegroups.com --cc math-nuts@swtch.com 99999 |
| 338 | </pre> |
| 339 | |
| 340 | <p>to achieve the same effect.</p> |
| 341 | |
| 342 | <p>Note that <code>-r</code> and <code>--cc</code> cannot be spelled <code>--r</code> or <code>-cc</code>.</p> |
| 343 | |
| 344 | <p> |
| 345 | If your change relates to an open issue, please add a comment to the issue |
| 346 | announcing your proposed fix, including a link to your CL. |
| 347 | </p> |
| 348 | |
| 349 | <h3>Reviewing code</h3> |
| 350 | |
| 351 | <p> |
| 352 | Running <code>hg mail</code> will send an email to you and the reviewers |
| 353 | asking them to visit the issue's URL and make comments on the change. |
| 354 | When done, the reviewer clicks “Publish and Mail comments” |
| 355 | to send comments back. |
| 356 | </p> |
| 357 | |
| 358 | |
| 359 | <h3>Revise and upload</h3> |
| 360 | |
| 361 | <p> |
| 362 | When you have revised the code and are ready for another round of review, |
| 363 | you can upload your change and send mail asking the reviewers to |
| 364 | please take another look (<code>PTAL</code>). Use the change list number |
| 365 | assigned during <code>hg change</code> |
| 366 | </p> |
| 367 | |
| 368 | <pre> |
| 369 | $ hg mail 99999 |
| 370 | </pre> |
| 371 | |
| 372 | |
| 373 | <p> |
| 374 | Or to upload your change without sending a notification, run |
| 375 | </p> |
| 376 | |
| 377 | <pre> |
| 378 | $ hg upload 99999 |
| 379 | </pre> |
| 380 | |
| 381 | <p> |
| 382 | You will probably revise your code in response to the reviewer comments. |
| 383 | You might also visit the code review web page and reply to the comments, |
| 384 | letting the reviewer know that you've addressed them or explain why you |
| 385 | haven't. When you're done replying, click “Publish and Mail comments” |
| 386 | to send the line-by-line replies and any other comments. |
| 387 | </p> |
| 388 | |
| 389 | <p> |
| 390 | The reviewer can comment on the new copy, and the process repeats. |
| 391 | The reviewer approves the change by replying with a mail that says |
| 392 | <code>LGTM</code>: looks good to me. |
| 393 | </p> |
| 394 | |
| 395 | <p> |
| 396 | You can see a list of your pending changes by running <code>hg pending</code> (<code>hg p</code> for short). |
| 397 | </p> |
| 398 | |
Dave Cheney | eda9590 | 2013-02-10 19:40:33 -0500 | [diff] [blame] | 399 | <h3>Adding or removing files from an existing change</h3> |
| 400 | |
Russ Cox | 38a41ee | 2009-11-06 10:04:22 -0800 | [diff] [blame] | 401 | <p> |
Shenghou Ma | 4c473c0 | 2012-10-11 23:33:57 +0800 | [diff] [blame] | 402 | If you need to re-edit the change description, or change the files included in the CL, |
Dave Cheney | eda9590 | 2013-02-10 19:40:33 -0500 | [diff] [blame] | 403 | run <code>hg change 99999</code>. |
Russ Cox | 38a41ee | 2009-11-06 10:04:22 -0800 | [diff] [blame] | 404 | </p> |
| 405 | |
| 406 | <p> |
Dave Cheney | eda9590 | 2013-02-10 19:40:33 -0500 | [diff] [blame] | 407 | Alternatively, you can use |
Rob Pike | 7ae41e8 | 2013-02-28 13:32:36 -0800 | [diff] [blame] | 408 | </p> |
Dave Cheney | eda9590 | 2013-02-10 19:40:33 -0500 | [diff] [blame] | 409 | |
| 410 | <pre> |
| 411 | $ hg file 99999 somefile |
| 412 | </pre> |
| 413 | |
| 414 | <p> |
| 415 | to add <code>somefile</code> to CL 99999, and |
| 416 | </p> |
| 417 | |
| 418 | <pre> |
| 419 | $ hg file -d 99999 somefile |
| 420 | </pre> |
| 421 | |
| 422 | <p> |
| 423 | to remove <code>somefile</code> from the CL. |
Russ Cox | c64469f | 2013-01-18 14:08:42 -0500 | [diff] [blame] | 424 | </p> |
| 425 | |
| 426 | <p> |
Dave Cheney | eda9590 | 2013-02-10 19:40:33 -0500 | [diff] [blame] | 427 | A file may only belong to a single active CL at a time. <code>hg file</code> |
| 428 | will issue a warning if a file is moved between changes. |
Russ Cox | 38a41ee | 2009-11-06 10:04:22 -0800 | [diff] [blame] | 429 | </p> |
| 430 | |
Russ Cox | 38a41ee | 2009-11-06 10:04:22 -0800 | [diff] [blame] | 431 | <h3>Synchronize your client</h3> |
| 432 | |
| 433 | <p>While you were working, others might have submitted changes |
| 434 | to the repository. To update your client, run</p> |
| 435 | |
| 436 | <pre> |
| 437 | $ hg sync |
| 438 | </pre> |
| 439 | |
| 440 | <p>(For Mercurial fans, <code>hg sync</code> runs <code>hg pull -u</code> |
| 441 | but then also synchronizes the local change list state against the new data.)</p> |
| 442 | |
| 443 | <p> |
| 444 | If files you were editing have changed, Mercurial does its best to merge the |
Russ Cox | 696e802 | 2009-11-07 18:56:00 -0800 | [diff] [blame] | 445 | remote changes into your local changes. It may leave some files to merge by hand. |
| 446 | </p> |
| 447 | |
| 448 | <p> |
| 449 | For example, suppose you have edited <code>flag_test.go</code> but |
| 450 | someone else has committed an independent change. |
| 451 | When you run <code>hg sync</code>, you will get the (scary-looking) output |
| 452 | (emphasis added): |
Russ Cox | 38a41ee | 2009-11-06 10:04:22 -0800 | [diff] [blame] | 453 | |
| 454 | <pre> |
Russ Cox | 696e802 | 2009-11-07 18:56:00 -0800 | [diff] [blame] | 455 | $ hg sync |
| 456 | adding changesets |
| 457 | adding manifests |
| 458 | adding file changes |
| 459 | added 1 changeset with 2 changes to 2 files |
| 460 | getting src/pkg/flag/flag.go |
| 461 | couldn't find merge tool hgmerge |
| 462 | merging src/pkg/flag/flag_test.go |
| 463 | warning: conflicts during merge. |
| 464 | <i>merging src/pkg/flag/flag_test.go failed!</i> |
| 465 | 1 file updated, 0 files merged, 0 files removed, 1 file unresolved |
| 466 | use 'hg resolve' to retry unresolved file merges |
Russ Cox | 830813f | 2009-11-08 21:08:27 -0800 | [diff] [blame] | 467 | $ |
Russ Cox | 38a41ee | 2009-11-06 10:04:22 -0800 | [diff] [blame] | 468 | </pre> |
| 469 | |
Russ Cox | 696e802 | 2009-11-07 18:56:00 -0800 | [diff] [blame] | 470 | <p> |
| 471 | The only important part in that transcript is the italicized line: |
| 472 | Mercurial failed to merge your changes with the independent change. |
| 473 | When this happens, Mercurial leaves both edits in the file, |
| 474 | marked by <code><<<<<<<</code> and |
| 475 | <code>>>>>>>></code>. |
| 476 | it is now your job to edit the file to combine them. |
| 477 | Continuing the example, searching for those strings in <code>flag_test.go</code> |
| 478 | might turn up: |
| 479 | </p> |
| 480 | |
| 481 | <pre> |
| 482 | VisitAll(visitor); |
| 483 | <<<<<<< local |
| 484 | if len(m) != 7 { |
| 485 | ======= |
| 486 | if len(m) != 8 { |
| 487 | >>>>>>> other |
| 488 | t.Error("VisitAll misses some flags"); |
| 489 | </pre> |
| 490 | |
| 491 | <p> |
| 492 | Mercurial doesn't show it, but suppose the original text that both edits |
| 493 | started with was 6; you added 1 and the other change added 2, |
Russ Cox | d55abfd | 2009-12-09 14:05:12 -0800 | [diff] [blame] | 494 | so the correct answer might now be 9. First, edit the section |
Russ Cox | 696e802 | 2009-11-07 18:56:00 -0800 | [diff] [blame] | 495 | to remove the markers and leave the correct code: |
| 496 | </p> |
| 497 | |
| 498 | <pre> |
| 499 | VisitAll(visitor); |
| 500 | if len(m) != 9 { |
| 501 | t.Error("VisitAll misses some flags"); |
| 502 | </pre> |
| 503 | |
| 504 | <p> |
Russ Cox | d55abfd | 2009-12-09 14:05:12 -0800 | [diff] [blame] | 505 | Then ask Mercurial to mark the conflict as resolved: |
Russ Cox | 696e802 | 2009-11-07 18:56:00 -0800 | [diff] [blame] | 506 | </p> |
| 507 | |
Russ Cox | d55abfd | 2009-12-09 14:05:12 -0800 | [diff] [blame] | 508 | <pre> |
| 509 | $ hg resolve -m flag_test.go |
| 510 | </pre> |
| 511 | |
Russ Cox | 696e802 | 2009-11-07 18:56:00 -0800 | [diff] [blame] | 512 | <p> |
Russ Cox | 830813f | 2009-11-08 21:08:27 -0800 | [diff] [blame] | 513 | If you had been editing the file, say for debugging, but do not |
| 514 | care to preserve your changes, you can run |
Russ Cox | 696e802 | 2009-11-07 18:56:00 -0800 | [diff] [blame] | 515 | <code>hg revert flag_test.go</code> to abandon your |
Russ Cox | d55abfd | 2009-12-09 14:05:12 -0800 | [diff] [blame] | 516 | changes, but you may still need to run |
| 517 | <code>hg resolve -m</code> to mark the conflict resolved. |
Russ Cox | 696e802 | 2009-11-07 18:56:00 -0800 | [diff] [blame] | 518 | </p> |
| 519 | |
Dave Cheney | eda9590 | 2013-02-10 19:40:33 -0500 | [diff] [blame] | 520 | <h3>Reviewing code by others</h3> |
| 521 | |
| 522 | <p> |
| 523 | You can import a CL proposed by someone else into your local Mercurial client |
| 524 | by using the <code>hg clpatch</code> command. Running |
| 525 | </p> |
| 526 | |
| 527 | <pre> |
| 528 | $ hg clpatch 99999 |
| 529 | </pre> |
| 530 | |
| 531 | <p> |
| 532 | will apply the latest diff for CL 99999 to your working copy. If any of the |
| 533 | files referenced in CL 99999 have local modifications, <code>clpatch</code> |
| 534 | will refuse to apply the whole diff. Once applied, CL 99999 will show up in |
| 535 | the output of <code>hg pending</code> and others. |
| 536 | </p> |
| 537 | |
| 538 | <p> |
| 539 | To revert a CL you have applied locally, use the <code>hg revert</code> |
| 540 | command. Running |
| 541 | </p> |
| 542 | |
| 543 | <pre> |
| 544 | $ hg revert @99999 |
| 545 | </pre> |
| 546 | |
| 547 | <p> |
| 548 | will revert any files mentioned on CL 99999 to their original state. This can |
| 549 | be an effective way of reverting one CL revision and applying another. |
| 550 | </p> |
| 551 | |
| 552 | <p> |
| 553 | Once the CL has been submitted, the next time you run <code>hg sync</code> |
| 554 | it will be removed from your local pending list. Occasionally the pending list |
Oling Cat | aecbcd0 | 2013-02-15 14:01:12 +1100 | [diff] [blame] | 555 | can get out of sync leaving stale references to closed or abandoned CLs. |
Dave Cheney | eda9590 | 2013-02-10 19:40:33 -0500 | [diff] [blame] | 556 | You can use <code>hg change -D 99999</code> to remove the reference to CL 99999. |
Oling Cat | aecbcd0 | 2013-02-15 14:01:12 +1100 | [diff] [blame] | 557 | </p> |
Dave Cheney | eda9590 | 2013-02-10 19:40:33 -0500 | [diff] [blame] | 558 | |
Russ Cox | 5facb84 | 2009-12-09 14:39:41 -0800 | [diff] [blame] | 559 | <h3>Submit the change after the review</h3> |
Russ Cox | 38a41ee | 2009-11-06 10:04:22 -0800 | [diff] [blame] | 560 | |
| 561 | <p> |
Russ Cox | 5facb84 | 2009-12-09 14:39:41 -0800 | [diff] [blame] | 562 | After the code has been <code>LGTM</code>'ed, it is time to submit |
Oling Cat | aecbcd0 | 2013-02-15 14:01:12 +1100 | [diff] [blame] | 563 | it to the Mercurial repository. |
Dave Cheney | eda9590 | 2013-02-10 19:40:33 -0500 | [diff] [blame] | 564 | </p> |
| 565 | |
| 566 | <p> |
| 567 | If you are not a committer, you cannot submit the change directly. |
| 568 | Instead a committer, usually the reviewer who said <code>LGTM</code>, |
| 569 | will run: |
| 570 | </p> |
| 571 | |
| 572 | <pre> |
| 573 | $ hg clpatch 99999 |
| 574 | $ hg submit 99999 |
| 575 | </pre> |
| 576 | |
Oling Cat | aecbcd0 | 2013-02-15 14:01:12 +1100 | [diff] [blame] | 577 | <p> |
Dave Cheney | eda9590 | 2013-02-10 19:40:33 -0500 | [diff] [blame] | 578 | The <code>submit</code> command submits the code. You will be listed as the |
| 579 | author, but the change message will also indicate who the committer was. |
| 580 | Your local client will notice that the change has been submitted |
| 581 | when you next run <code>hg sync</code>. |
| 582 | </p> |
| 583 | |
| 584 | <p> |
Russ Cox | 38a41ee | 2009-11-06 10:04:22 -0800 | [diff] [blame] | 585 | If you are a committer, you can run: |
| 586 | </p> |
| 587 | |
| 588 | <pre> |
| 589 | $ hg submit 99999 |
| 590 | </pre> |
| 591 | |
| 592 | <p> |
| 593 | This checks the change into the repository. |
| 594 | The change description will include a link to the code review, |
| 595 | and the code review will be updated with a link to the change |
| 596 | in the repository. |
| 597 | </p> |
| 598 | |
| 599 | <p> |
| 600 | If your local copy of the repository is out of date, |
Oling Cat | aecbcd0 | 2013-02-15 14:01:12 +1100 | [diff] [blame] | 601 | <code>hg submit</code> will refuse the change: |
Russ Cox | 38a41ee | 2009-11-06 10:04:22 -0800 | [diff] [blame] | 602 | </p> |
| 603 | |
| 604 | <pre> |
Russ Cox | d55abfd | 2009-12-09 14:05:12 -0800 | [diff] [blame] | 605 | $ hg submit 99999 |
Russ Cox | 38a41ee | 2009-11-06 10:04:22 -0800 | [diff] [blame] | 606 | local repository out of date; must sync before submit |
| 607 | </pre> |
| 608 | |
Robert Hencke | 75ba181 | 2014-04-25 20:09:04 -0700 | [diff] [blame] | 609 | <h3>More information</h3> |
| 610 | |
| 611 | <p> |
| 612 | In addition to the information here, the Go community maintains a <a href="https://code.google.com/p/go-wiki/wiki/CodeReview">CodeReview</a> wiki page. |
| 613 | Feel free to contribute to this page as you learn the review process. |
| 614 | </p> |
| 615 | |
Dave Cheney | eda9590 | 2013-02-10 19:40:33 -0500 | [diff] [blame] | 616 | <h2 id="copyright">Copyright</h2> |
Russ Cox | 38a41ee | 2009-11-06 10:04:22 -0800 | [diff] [blame] | 617 | |
Russ Cox | d55abfd | 2009-12-09 14:05:12 -0800 | [diff] [blame] | 618 | <p>Files in the Go repository don't list author names, |
| 619 | both to avoid clutter and to avoid having to keep the lists up to date. |
Brad Fitzpatrick | 967901a | 2014-03-05 14:09:03 -0800 | [diff] [blame] | 620 | Instead, your name will appear in the <a href="https://code.google.com/p/go/source/list">Mercurial change log</a> |
Russ Cox | d55abfd | 2009-12-09 14:05:12 -0800 | [diff] [blame] | 621 | and in the <a href="/CONTRIBUTORS"><code>CONTRIBUTORS</code></a> file |
| 622 | and perhaps the <a href="/AUTHORS"><code>AUTHORS</code></a> file. |
| 623 | </p> |
| 624 | |
| 625 | <p>The <a href="/CONTRIBUTORS"><code>CONTRIBUTORS</code></a> file |
| 626 | defines who the Go contributors—the people—are; |
Caleb Spare | 41f32e0 | 2013-01-06 22:44:16 -0500 | [diff] [blame] | 627 | the <a href="/AUTHORS"><code>AUTHORS</code></a> file defines |
Russ Cox | d55abfd | 2009-12-09 14:05:12 -0800 | [diff] [blame] | 628 | who “The Go Authors”—the copyright holders—are. |
| 629 | The Go developers at Google will update these files when submitting |
| 630 | your first change. |
| 631 | In order for them to do that, you need to have completed one of the |
| 632 | contributor license agreements: |
| 633 | <ul> |
| 634 | <li> |
Brad Fitzpatrick | 967901a | 2014-03-05 14:09:03 -0800 | [diff] [blame] | 635 | If you are the copyright holder, you will need to agree to the |
| 636 | <a href="https://developers.google.com/open-source/cla/individual">individual |
Russ Cox | d55abfd | 2009-12-09 14:05:12 -0800 | [diff] [blame] | 637 | contributor license agreement</a>, which can be completed online. |
| 638 | </li> |
| 639 | <li> |
| 640 | If your organization is the copyright holder, the organization |
Brad Fitzpatrick | 967901a | 2014-03-05 14:09:03 -0800 | [diff] [blame] | 641 | will need to agree to the |
| 642 | <a href="https://developers.google.com/open-source/cla/corporate">corporate |
| 643 | contributor license agreement</a>. |
Russ Cox | d55abfd | 2009-12-09 14:05:12 -0800 | [diff] [blame] | 644 | (If the copyright holder for your code has already completed the |
| 645 | agreement in connection with another Google open source project, |
| 646 | it does not need to be completed again.) |
| 647 | </li> |
| 648 | </ul> |
| 649 | |
| 650 | <p> |
| 651 | This rigmarole needs to be done only for your first submission. |
| 652 | </p> |
| 653 | |
| 654 | <p>Code that you contribute should use the standard copyright header:</p> |
Russ Cox | 38a41ee | 2009-11-06 10:04:22 -0800 | [diff] [blame] | 655 | |
| 656 | <pre> |
David Symonds | 8f3fc547 | 2014-01-01 00:00:22 +1100 | [diff] [blame] | 657 | // Copyright 2014 The Go Authors. All rights reserved. |
Russ Cox | 38a41ee | 2009-11-06 10:04:22 -0800 | [diff] [blame] | 658 | // Use of this source code is governed by a BSD-style |
| 659 | // license that can be found in the LICENSE file. |
| 660 | </pre> |
Dave Cheney | eda9590 | 2013-02-10 19:40:33 -0500 | [diff] [blame] | 661 | |
| 662 | <p> |
| 663 | Files in the repository are copyright the year they are added. It is not |
| 664 | necessary to update the copyright year on files that you change. |
| 665 | </p> |