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