mirror of
https://github.com/golang/go
synced 2024-11-24 07:40:17 -07:00
8f3fc547d0
R=adg, djd, mpvl, nigeltao CC=golang-codereviews https://golang.org/cl/46660043
648 lines
18 KiB
HTML
648 lines
18 KiB
HTML
<!--{
|
|
"Title": "Contribution Guidelines"
|
|
}-->
|
|
|
|
<h2 id="Introduction">Introduction</h2>
|
|
|
|
<p>
|
|
This document explains how to contribute changes to the Go project.
|
|
It assumes you have installed Go using the
|
|
<a href="/doc/install/source">installation instructions</a> and
|
|
have <a href="code.html">written and tested your code</a>.
|
|
(Note that the <code>gccgo</code> frontend lives elsewhere;
|
|
see <a href="gccgo_contribute.html">Contributing to gccgo</a>.)
|
|
</p>
|
|
|
|
<h2 id="Design">Discuss your design</h2>
|
|
|
|
<p>
|
|
The project welcomes submissions but please let everyone know what
|
|
you're working on if you want it to become part of the main repository.
|
|
</p>
|
|
|
|
<p>
|
|
Before undertaking to write something new for the Go project, send
|
|
mail to the <a href="http://groups.google.com/group/golang-nuts">mailing
|
|
list</a> to discuss what you plan to do. This gives everyone a
|
|
chance to validate the design, helps prevent duplication of effort,
|
|
and ensures that the idea fits inside the goals for the language
|
|
and tools. It also guarantees that the design is sound before code
|
|
is written; the code review tool is not the place for high-level
|
|
discussions.
|
|
</p>
|
|
|
|
<p>
|
|
In short, send mail before you code.
|
|
And don't start the discussion by mailing a change list!
|
|
</p>
|
|
|
|
<h2 id="Testing">Testing redux</h2>
|
|
|
|
<p>
|
|
You've <a href="code.html">written and tested your code</a>, but
|
|
before sending code out for review, run all the tests for the whole
|
|
tree to make sure the changes don't break other packages or programs:
|
|
</p>
|
|
|
|
<pre>
|
|
cd $GOROOT/src
|
|
./all.bash # On Windows, run all.bat
|
|
</pre>
|
|
|
|
<p>
|
|
After running for a while, the command should print "<code>ALL TESTS PASSED</code>".
|
|
</p>
|
|
|
|
<h2 id="Code_review">Code review</h2>
|
|
|
|
<p>
|
|
Changes to Go must be reviewed before they are submitted,
|
|
no matter who makes the change.
|
|
(In exceptional cases, such as fixing a build, the review can
|
|
follow shortly after submitting.)
|
|
A Mercurial extension helps manage the code review process.
|
|
The extension is included in the Go source tree but needs
|
|
to be added to your Mercurial configuration.
|
|
</p>
|
|
|
|
<h3>Caveat for Mercurial aficionados</h3>
|
|
|
|
<p>
|
|
<i>Using Mercurial with the code review extension is not the same
|
|
as using standard Mercurial.</i>
|
|
</p>
|
|
|
|
<p>
|
|
The Go repository is maintained as a single line of reviewed changes;
|
|
we prefer to avoid the complexity of Mercurial's arbitrary change graph.
|
|
The code review extension helps here: its <code>hg submit</code> command
|
|
automatically checks for and warns about the local repository
|
|
being out of date compared to the remote one.
|
|
The <code>hg submit</code> command also verifies other
|
|
properties about the Go repository.
|
|
For example,
|
|
it checks that Go code being checked in is formatted in the standard style,
|
|
as defined by <a href="/cmd/gofmt">gofmt</a>,
|
|
and it checks that the author of the code is properly recorded for
|
|
<a href="#copyright">copyright purposes</a>.
|
|
</p>
|
|
|
|
<p>
|
|
To help ensure changes are only created by <code>hg submit</code>,
|
|
the code review extension disables the standard <code>hg commit</code>
|
|
command.
|
|
</p>
|
|
|
|
<h3>Configure the extension</h3>
|
|
|
|
<p>Edit <code>$GOROOT/.hg/hgrc</code> to add:</p>
|
|
|
|
<pre>
|
|
[extensions]
|
|
codereview = $GOROOT/lib/codereview/codereview.py
|
|
|
|
[ui]
|
|
username = Your Name <you@server.dom>
|
|
</pre>
|
|
|
|
<p>
|
|
The <code>username</code> information will not be used unless
|
|
you are a committer (see below), but Mercurial complains if it is missing.
|
|
</p>
|
|
|
|
<h3>Understanding the extension</h3>
|
|
|
|
<p>After adding the code review extension, you can run</p>
|
|
|
|
<pre>
|
|
$ hg help codereview
|
|
</pre>
|
|
|
|
<p>to learn more about its commands. To learn about a specific code-review-specific
|
|
command such as <code>change</code>, run</p>
|
|
|
|
<pre>
|
|
$ hg help change
|
|
</pre>
|
|
|
|
<p>
|
|
As the codereview extension is only enabled for your checkout
|
|
in <code>$GOROOT</code>, the remainder of this document assumes you
|
|
are inside <code>$GOROOT</code> when issuing commands.
|
|
</p>
|
|
|
|
<p>
|
|
Windows users may need to perform extra steps to get the code review
|
|
extension working. See the
|
|
<a href="https://code.google.com/p/go-wiki/wiki/CodeReview">CodeReview page</a>
|
|
on the <a href="http://code.google.com/p/go-wiki/wiki">Go Wiki</a> for details.
|
|
</p>
|
|
|
|
<h3>Log in to the code review site.</h3>
|
|
|
|
<p>
|
|
The code review server uses a Google Account to authenticate.
|
|
(If you can use the account to
|
|
<a href="https://www.google.com/accounts/Login?hl=en&continue=http://www.google.com/">sign in at google.com</a>,
|
|
you can use it to sign in to the code review server.)
|
|
The email address you use on the Code Review site
|
|
will be recorded in the <a href="http://code.google.com/p/go/source/list">Mercurial change log</a>
|
|
and in the <a href="/CONTRIBUTORS"><code>CONTRIBUTORS</code></a> file.
|
|
You can <a href="https://www.google.com/accounts/NewAccount">create a Google Account</a>
|
|
associated with any address where you receive email.
|
|
If you've enabled the two-step verification feature, don't forget to generate an
|
|
application-specific password and use that when prompted for a password.
|
|
</p>
|
|
|
|
<pre>
|
|
$ cd $GOROOT
|
|
$ hg code-login
|
|
Email (login for uploading to codereview.appspot.com): rsc@golang.org
|
|
Password for rsc@golang.org:
|
|
|
|
Saving authentication cookies to /Users/rsc/.codereview_upload_cookies_codereview.appspot.com
|
|
</pre>
|
|
|
|
<h3>Configure your account settings.</h3>
|
|
|
|
<p>Edit your <a href="http://codereview.appspot.com/settings">code review settings</a>.
|
|
Grab a nickname.
|
|
Many people prefer to set the Context option to
|
|
“Whole file” to see more context when reviewing changes.
|
|
</p>
|
|
|
|
<p>Once you have chosen a nickname in the settings page, others
|
|
can use that nickname as a shorthand for naming reviewers and the CC list.
|
|
For example, <code>rsc</code> is an alias for <code>rsc@golang.org</code>.
|
|
</p>
|
|
|
|
<h3>Switch to the default branch</h3>
|
|
|
|
<p>
|
|
Most Go installations use a release branch, but new changes should
|
|
only be made to the default branch. (They may be applied later to a release
|
|
branch as part of the release process.)
|
|
Before making a change, make sure you use the default branch:
|
|
</p>
|
|
|
|
<pre>
|
|
$ hg update default
|
|
</pre>
|
|
|
|
<h3>Make a change</h3>
|
|
|
|
<p>
|
|
The entire checked-out tree is writable.
|
|
If you need to edit files, just edit them: Mercurial will figure out which ones changed.
|
|
You do need to inform Mercurial of added, removed, copied, or renamed files,
|
|
by running
|
|
<code>hg add</code>,
|
|
<code>hg rm</code>,
|
|
<code>hg cp</code>,
|
|
or
|
|
<code>hg mv</code>.
|
|
</p>
|
|
|
|
<p>When you are ready to send a change out for review, run</p>
|
|
|
|
<pre>
|
|
$ hg change
|
|
</pre>
|
|
|
|
<p>from any directory in your Go repository.
|
|
Mercurial will open a change description file in your editor.
|
|
(It uses the editor named by the <code>$EDITOR</code> environment variable, <code>vi</code> by default.)
|
|
The file will look like:
|
|
</p>
|
|
|
|
<pre>
|
|
# Change list.
|
|
# Lines beginning with # are ignored.
|
|
# Multi-line values should be indented.
|
|
|
|
Reviewer:
|
|
CC:
|
|
|
|
Description:
|
|
<enter description here>
|
|
|
|
Files:
|
|
src/pkg/math/sin.go
|
|
src/pkg/math/tan.go
|
|
src/pkg/regexp/regexp.go
|
|
</pre>
|
|
|
|
<p>
|
|
The <code>Reviewer</code> line lists the reviewers assigned
|
|
to this change, and the <code>CC</code> line lists people to
|
|
notify about the change.
|
|
These can be code review nicknames or arbitrary email addresses.
|
|
Unless explicitly told otherwise, such as in the discussion leading
|
|
up to sending in the change list, leave the reviewer field blank.
|
|
This means that the
|
|
<a href="http://groups.google.com/group/golang-codereviews">golang-codereviews@googlegroups.com</a>
|
|
mailing list will be used as the reviewer.
|
|
</p>
|
|
|
|
<p>
|
|
Replace “<code><enter description here></code>”
|
|
with a description of your change.
|
|
The first line of the change description is conventionally a one-line
|
|
summary of the change, prefixed by the primary affected package,
|
|
and is used as the subject for code review mail; the rest of the
|
|
description elaborates.
|
|
</p>
|
|
|
|
<p>
|
|
The <code>Files</code> section lists all the modified files
|
|
in your client.
|
|
It is best to keep unrelated changes in different change lists.
|
|
In this example, we can include just the changes to package <code>math</code>
|
|
by deleting the line mentioning <code>regexp.go</code>.
|
|
</p>
|
|
|
|
<p>
|
|
After editing, the template might now read:
|
|
</p>
|
|
|
|
<pre>
|
|
# Change list.
|
|
# Lines beginning with # are ignored.
|
|
# Multi-line values should be indented.
|
|
|
|
Reviewer: golang-codereviews@googlegroups.com
|
|
CC: math-nuts@swtch.com
|
|
|
|
Description:
|
|
math: improved Sin, Cos and Tan precision for very large arguments.
|
|
|
|
See Bimmler and Shaney, ``Extreme sinusoids,'' J. Math 3(14).
|
|
Fixes issue 159.
|
|
|
|
Files:
|
|
src/pkg/math/sin.go
|
|
src/pkg/math/tan.go
|
|
</pre>
|
|
|
|
<p>
|
|
The special sentence “Fixes issue 159.” associates
|
|
the change with issue 159 in the <a href="http://code.google.com/p/go/issues/list">Go issue tracker</a>.
|
|
When this change is eventually submitted, the issue
|
|
tracker will automatically mark the issue as fixed.
|
|
(These conventions are described in detail by the
|
|
<a href="http://code.google.com/p/support/wiki/IssueTracker#Integration_with_version_control">Google Project Hosting Issue Tracker documentation</a>.)
|
|
</p>
|
|
|
|
<p>
|
|
Save the file and exit the editor.</p>
|
|
|
|
<p>
|
|
The code review server assigns your change an issue number and URL,
|
|
which <code>hg change</code> will print, something like:
|
|
</p>
|
|
|
|
<pre>
|
|
CL created: http://codereview.appspot.com/99999
|
|
</pre>
|
|
|
|
<h3>Adding or removing files from an existing change</h3>
|
|
|
|
<p>
|
|
If you need to re-edit the change description, or change the files included in the CL,
|
|
run <code>hg change 99999</code>.
|
|
</p>
|
|
|
|
<p>
|
|
Alternatively, you can use
|
|
</p>
|
|
|
|
<pre>
|
|
$ hg file 99999 somefile
|
|
</pre>
|
|
|
|
<p>
|
|
to add <code>somefile</code> to CL 99999, and
|
|
</p>
|
|
|
|
<pre>
|
|
$ hg file -d 99999 somefile
|
|
</pre>
|
|
|
|
<p>
|
|
to remove <code>somefile</code> from the CL.
|
|
</p>
|
|
|
|
<p>
|
|
A file may only belong to a single active CL at a time. <code>hg file</code>
|
|
will issue a warning if a file is moved between changes.
|
|
</p>
|
|
|
|
<h3>Synchronize your client</h3>
|
|
|
|
<p>While you were working, others might have submitted changes
|
|
to the repository. To update your client, run</p>
|
|
|
|
<pre>
|
|
$ hg sync
|
|
</pre>
|
|
|
|
<p>(For Mercurial fans, <code>hg sync</code> runs <code>hg pull -u</code>
|
|
but then also synchronizes the local change list state against the new data.)</p>
|
|
|
|
<p>
|
|
If files you were editing have changed, Mercurial does its best to merge the
|
|
remote changes into your local changes. It may leave some files to merge by hand.
|
|
</p>
|
|
|
|
<p>
|
|
For example, suppose you have edited <code>flag_test.go</code> but
|
|
someone else has committed an independent change.
|
|
When you run <code>hg sync</code>, you will get the (scary-looking) output
|
|
(emphasis added):
|
|
|
|
<pre>
|
|
$ hg sync
|
|
adding changesets
|
|
adding manifests
|
|
adding file changes
|
|
added 1 changeset with 2 changes to 2 files
|
|
getting src/pkg/flag/flag.go
|
|
couldn't find merge tool hgmerge
|
|
merging src/pkg/flag/flag_test.go
|
|
warning: conflicts during merge.
|
|
<i>merging src/pkg/flag/flag_test.go failed!</i>
|
|
1 file updated, 0 files merged, 0 files removed, 1 file unresolved
|
|
use 'hg resolve' to retry unresolved file merges
|
|
$
|
|
</pre>
|
|
|
|
<p>
|
|
The only important part in that transcript is the italicized line:
|
|
Mercurial failed to merge your changes with the independent change.
|
|
When this happens, Mercurial leaves both edits in the file,
|
|
marked by <code><<<<<<<</code> and
|
|
<code>>>>>>>></code>.
|
|
it is now your job to edit the file to combine them.
|
|
Continuing the example, searching for those strings in <code>flag_test.go</code>
|
|
might turn up:
|
|
</p>
|
|
|
|
<pre>
|
|
VisitAll(visitor);
|
|
<<<<<<< local
|
|
if len(m) != 7 {
|
|
=======
|
|
if len(m) != 8 {
|
|
>>>>>>> other
|
|
t.Error("VisitAll misses some flags");
|
|
</pre>
|
|
|
|
<p>
|
|
Mercurial doesn't show it, but suppose the original text that both edits
|
|
started with was 6; you added 1 and the other change added 2,
|
|
so the correct answer might now be 9. First, edit the section
|
|
to remove the markers and leave the correct code:
|
|
</p>
|
|
|
|
<pre>
|
|
VisitAll(visitor);
|
|
if len(m) != 9 {
|
|
t.Error("VisitAll misses some flags");
|
|
</pre>
|
|
|
|
<p>
|
|
Then ask Mercurial to mark the conflict as resolved:
|
|
</p>
|
|
|
|
<pre>
|
|
$ hg resolve -m flag_test.go
|
|
</pre>
|
|
|
|
<p>
|
|
If you had been editing the file, say for debugging, but do not
|
|
care to preserve your changes, you can run
|
|
<code>hg revert flag_test.go</code> to abandon your
|
|
changes, but you may still need to run
|
|
<code>hg resolve -m</code> to mark the conflict resolved.
|
|
</p>
|
|
|
|
<h3>Mail the change for review</h3>
|
|
|
|
<p>
|
|
Creating or uploading the change uploads a copy of the diff to the code review server,
|
|
but it does not notify anyone about it. To do that, you need to run <code>hg mail</code>
|
|
(see below).
|
|
</p>
|
|
|
|
<p>To send out a change for review, run <code>hg mail</code> using the change list number
|
|
assigned during <code>hg change</code>:</p>
|
|
|
|
<pre>
|
|
$ hg mail 99999
|
|
</pre>
|
|
|
|
<p>You can add to the <code>Reviewer:</code> and <code>CC:</code> lines
|
|
using the <code>-r</code> or <code>--cc</code> options.
|
|
In the above example, we could have left the <code>Reviewer</code> and <code>CC</code>
|
|
lines blank and then run:
|
|
</p>
|
|
|
|
<pre>
|
|
$ hg mail -r golang-codereviews@googlegroups.com --cc math-nuts@swtch.com 99999
|
|
</pre>
|
|
|
|
<p>to achieve the same effect.</p>
|
|
|
|
<p>Note that <code>-r</code> and <code>--cc</code> cannot be spelled <code>--r</code> or <code>-cc</code>.</p>
|
|
|
|
<p>
|
|
If your change relates to an open issue, please add a comment to the issue
|
|
announcing your proposed fix, including a link to your CL.
|
|
</p>
|
|
|
|
<h3>Reviewing code</h3>
|
|
|
|
<p>
|
|
Running <code>hg mail</code> will send an email to you and the reviewers
|
|
asking them to visit the issue's URL and make comments on the change.
|
|
When done, the reviewer clicks “Publish and Mail comments”
|
|
to send comments back.
|
|
</p>
|
|
|
|
|
|
<h3>Revise and upload</h3>
|
|
|
|
<p>
|
|
You will probably revise your code in response to the reviewer comments. When
|
|
you have done this, you can upload your change to the code review server
|
|
without sending a notification by running <code>hg upload</code> using the change
|
|
list number assigned during <code>hg change</code>
|
|
</p>
|
|
|
|
<pre>
|
|
$ hg upload 99999
|
|
</pre>
|
|
|
|
<p>
|
|
When you have revised the code and are ready for another round of review, run
|
|
</p>
|
|
|
|
<pre>
|
|
$ hg mail 99999
|
|
</pre>
|
|
|
|
<p>again to upload the latest copy and send mail asking the reviewers to please take another look
|
|
(<code>PTAL</code>).
|
|
You might also visit the code review web page and reply to the comments,
|
|
letting the reviewer know that you've addressed them or explain why you
|
|
haven't. When you're done replying, click “Publish and Mail comments”
|
|
to send the line-by-line replies and any other comments.
|
|
</p>
|
|
<p>
|
|
The reviewer can comment on the new copy, and the process repeats.
|
|
The reviewer approves the change by replying with a mail that says
|
|
<code>LGTM</code>: looks good to me.
|
|
</p>
|
|
|
|
<p>
|
|
You can see a list of your pending changes by running <code>hg pending</code> (<code>hg p</code> for short).
|
|
</p>
|
|
|
|
<h3>Reviewing code by others</h3>
|
|
|
|
<p>
|
|
You can import a CL proposed by someone else into your local Mercurial client
|
|
by using the <code>hg clpatch</code> command. Running
|
|
</p>
|
|
|
|
<pre>
|
|
$ hg clpatch 99999
|
|
</pre>
|
|
|
|
<p>
|
|
will apply the latest diff for CL 99999 to your working copy. If any of the
|
|
files referenced in CL 99999 have local modifications, <code>clpatch</code>
|
|
will refuse to apply the whole diff. Once applied, CL 99999 will show up in
|
|
the output of <code>hg pending</code> and others.
|
|
</p>
|
|
|
|
<p>
|
|
To revert a CL you have applied locally, use the <code>hg revert</code>
|
|
command. Running
|
|
</p>
|
|
|
|
<pre>
|
|
$ hg revert @99999
|
|
</pre>
|
|
|
|
<p>
|
|
will revert any files mentioned on CL 99999 to their original state. This can
|
|
be an effective way of reverting one CL revision and applying another.
|
|
</p>
|
|
|
|
<p>
|
|
Once the CL has been submitted, the next time you run <code>hg sync</code>
|
|
it will be removed from your local pending list. Occasionally the pending list
|
|
can get out of sync leaving stale references to closed or abandoned CLs.
|
|
You can use <code>hg change -D 99999</code> to remove the reference to CL 99999.
|
|
</p>
|
|
|
|
<h3>Submit the change after the review</h3>
|
|
|
|
<p>
|
|
After the code has been <code>LGTM</code>'ed, it is time to submit
|
|
it to the Mercurial repository.
|
|
</p>
|
|
|
|
<p>
|
|
If you are not a committer, you cannot submit the change directly.
|
|
Instead a committer, usually the reviewer who said <code>LGTM</code>,
|
|
will run:
|
|
</p>
|
|
|
|
<pre>
|
|
$ hg clpatch 99999
|
|
$ hg submit 99999
|
|
</pre>
|
|
|
|
<p>
|
|
The <code>submit</code> command submits the code. You will be listed as the
|
|
author, but the change message will also indicate who the committer was.
|
|
Your local client will notice that the change has been submitted
|
|
when you next run <code>hg sync</code>.
|
|
</p>
|
|
|
|
<p>
|
|
If you are a committer, you can run:
|
|
</p>
|
|
|
|
<pre>
|
|
$ hg submit 99999
|
|
</pre>
|
|
|
|
<p>
|
|
This checks the change into the repository.
|
|
The change description will include a link to the code review,
|
|
and the code review will be updated with a link to the change
|
|
in the repository.
|
|
</p>
|
|
|
|
<p>
|
|
If your local copy of the repository is out of date,
|
|
<code>hg submit</code> will refuse the change:
|
|
</p>
|
|
|
|
<pre>
|
|
$ hg submit 99999
|
|
local repository out of date; must sync before submit
|
|
</pre>
|
|
|
|
<h2 id="copyright">Copyright</h2>
|
|
|
|
<p>Files in the Go repository don't list author names,
|
|
both to avoid clutter and to avoid having to keep the lists up to date.
|
|
Instead, your name will appear in the <a href="http://code.google.com/p/go/source/list">Mercurial change log</a>
|
|
and in the <a href="/CONTRIBUTORS"><code>CONTRIBUTORS</code></a> file
|
|
and perhaps the <a href="/AUTHORS"><code>AUTHORS</code></a> file.
|
|
</p>
|
|
|
|
<p>The <a href="/CONTRIBUTORS"><code>CONTRIBUTORS</code></a> file
|
|
defines who the Go contributors—the people—are;
|
|
the <a href="/AUTHORS"><code>AUTHORS</code></a> file defines
|
|
who “The Go Authors”—the copyright holders—are.
|
|
The Go developers at Google will update these files when submitting
|
|
your first change.
|
|
In order for them to do that, you need to have completed one of the
|
|
contributor license agreements:
|
|
<ul>
|
|
<li>
|
|
If you are the copyright holder, you will need to agree to
|
|
the <a href="http://code.google.com/legal/individual-cla-v1.0.html">individual
|
|
contributor license agreement</a>, which can be completed online.
|
|
</li>
|
|
<li>
|
|
If your organization is the copyright holder, the organization
|
|
will need to agree to the <a href="http://code.google.com/legal/corporate-cla-v1.0.html">corporate contributor license agreement</a>.
|
|
(If the copyright holder for your code has already completed the
|
|
agreement in connection with another Google open source project,
|
|
it does not need to be completed again.)
|
|
</li>
|
|
</ul>
|
|
|
|
<p>
|
|
This rigmarole needs to be done only for your first submission.
|
|
</p>
|
|
|
|
<p>Code that you contribute should use the standard copyright header:</p>
|
|
|
|
<pre>
|
|
// Copyright 2014 The Go Authors. All rights reserved.
|
|
// Use of this source code is governed by a BSD-style
|
|
// license that can be found in the LICENSE file.
|
|
</pre>
|
|
|
|
<p>
|
|
Files in the repository are copyright the year they are added. It is not
|
|
necessary to update the copyright year on files that you change.
|
|
</p>
|