From 38a41eec67e5a630385f50c0ac58d5e10e153cb5 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Fri, 6 Nov 2009 10:04:22 -0800 Subject: [PATCH] contribute.html R=r, iant CC=go-dev http://go/go-review/1022007 --- doc/contribute.html | 538 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 515 insertions(+), 23 deletions(-) diff --git a/doc/contribute.html b/doc/contribute.html index 8ab7e0d2238..633d1b1e4a9 100644 --- a/doc/contribute.html +++ b/doc/contribute.html @@ -1,36 +1,528 @@ -

TODO(go-dev): Write this document

+ + +

Introduction

-Have to work on the tools first. +This document explains how to write a new package, +how to test code, and how to contribute changes to the Go project. +It assumes you have installed Go and Mercurial using the +installation instructions. +

-Text previously from the FAQ placed here for safekeeping. +Before embarking on a significant change to an existing +package or the creation of a major new package, +it's a good idea to send mail to the mailing list +to let people know what you are thinking of doing. +Doing so helps avoid duplication of effort and +enables discussions about design before much code +has been written. +

-
    -
  1. If it's a significant change, discuss on the mailing list before embarking. +

    Creating a new package

    -
  2. Check out the Go source code files. The library sources are in go/src/pkg. +

    +The source code for the package with import path +x/y is, by convention, kept in the +directory $GOROOT/src/pkg/x/y. +

    -
  3. Make changes; add tests as appropriate. Try to follow existing style, - including tabs for indentation, and no trailing whitespace. In - documentation comments for public declarations, use full sentences - and begin with the name of the thing being described, because godoc - (or other tools) may someday display these comments out of context. +

    Makefile

    -
  4. Write the Makefile by following existing examples. +

    +It would be nice to have Go-specific tools that +inspect the source files to determine what to build and in +what order, but for now, Go uses GNU make. +Thus, the first file to create in a new package directory is +usually the Makefile. +The basic form is illustrated by src/pkg/container/vector/Makefile: +

    -
  5. Run make and make test in the affected - directories. +
    +include $(GOROOT)/src/Make.$(GOARCH)
 
-
  6. If you have added a new dependency, you may need to cd go/src/lib;
-    ./deps.bash to update the Make.deps file included in the Makefile.
-    For a new component, update the Makefile and then run
-    deps.bash.
-
  7. cd go/src; ./all.bash
+TARG=container/vector
+GOFILES=\
+	intvector.go\
+	stringvector.go\
+	vector.go\
+
+include $(GOROOT)/src/Make.pkg
+
    8. + +

    +The first and last lines include standard definitions and rules, +so that the body of the Makefile need only specify two variables. +

    + +

    +TARG is the target install path for the package, +the string that clients will use to import it. +This string should be the same as the directory +in which the Makefile appears, with the +$GOROOT/src/pkg/ removed. +

    + +

    +GOFILES is a list of source files to compile to +create the package. The trailing \ characters +allow the list to be split onto multiple lines +for easy sorting. +

    + +

    +After creating a new package directory, add it to the list in +$GOROOT/src/pkg/Makefile so that it +is included in the standard build. Then run: +

    +cd $GOROOT/src/pkg
+./deps.bash
+
    +

    +to update the dependency file Make.deps. +

    + +

    +If you change the imports of an existing package, +you do not need to edit $GOROOT/src/pkg/Makefile +but you will still need to run deps.bash as above. +

    + + +

    Go source files

    + +

    +The first statement in each of the source files listed in the Makefile +should be package name, where name +is the package's default name for imports. +(All files in a package must use the same name.) +Go's convention is that the package name is the last element of the +import path: the package imported as "crypto/rot13" +should be named rot13. +The Go tools impose a restriction that package names are unique +across all packages linked into a single binary, but that restriction +will be lifted soon. +

    + +

    +Go compiles all the source files in a package at once, so one file +can refer to constants, variables, types, and functions in another +file without special arrangement or declarations. +

    + +

    +Writing clean, idiomatic Go code is beyond the scope of this document. +Effective Go is an introduction to +that topic. +

    + +

    Testing

    + +

    +Go has a lightweight test framework known as gotest. +You write a test by creating a file with a name ending in _test.go +that contains functions named TestXXX with signature func (t *testing.T). +The test framework runs each such function; +if the function calls a failure function such as t.Error or t.Fail, the test is considered to have failed. +The gotest command documentation +and the testing package documentation give more detail. +

    + +

    +The *_test.go files should not be listed in the Makefile. +

    + +

    +To run the test, run either make test or gotest +(they are equivalent). +To run only the tests in a single test file, for instance one_test.go, +run gotest one_test.go. +

    + +

    +Before sending code out for review, make sure everything +still works and the dependencies are right: +

    + +
    +cd $GOROOT/src
+./all.bash
+
    + +

    +The final line printed by all.bash should be of the form: +

    + +
    +N known bugs; 0 unexpected bugs
+
    + +

    +The value of N varies over time, but the line must +say “0 unexpected bugs” and must not +add “test output differs.” +

    + +

    +Once your new code is tested and working, +it's time to get it reviewed and submitted. +

    + +

    Installing the code review extension

    + +

    +Changes to Go must be reviewed before they are submitted, +no matter who makes the change. +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. +

    + +

    +Using Mercurial with the code review extension is not the same +as using it normally. +

    + +

    +TODO(rsc): note here about model being different. +Do not use hg commit if you are using the Mercurial extension. +

    + +

    Configure the extension

    + +[NOTE FOR BEFORE LAUNCH: Read this instead.] + +

    Edit $GOROOT/.hg/hgrc to add:

    + +
    +[extensions]
+codereview = YOUR_GO_ROOT/lib/codereview/codereview.py
+
    + +

    Replace YOUR_GO_ROOT with the value of $GOROOT. +The Mercurial configuration file format does not allow environment variable substitution. +

    + +

    Log in to the code review site.

    + +[NOTE FOR BEFORE LAUNCH: Read this instead.] + +

    +The code review server uses a Google Account to authenticate. +(If you can use the account to +sign in at google.com, +you can use it to sign in to the code review server.) +

    + +
    +$ cd $GOROOT
+$ hg codereview-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
+
    + +

    Configure your account settings.

    + +

    Edit your code review settings. +Grab a nickname. +Many people refer to set the Context option to +“Whole file” to see more context when reviewing changes. +

    + +

    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, rsc is an alias for rsc@golang.org. +

    + +

    Changing code

    + +

    +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 +hg add, +hg rm, +hg cp, +or +hg mv. +

    + +

    Create a change

    + +

    When you are ready to send a change out for review, run

    + +
    +$ hg change
+
    + +

    from any directory in your Go repository. +Mercurial will open a change description file in your editor. +(It uses the editor named by the $EDITOR environment variable, vi by default.) +The file will look like: +

    + +
    +# 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
+
    + +

    +The Reviewer line lists the reviewers assigned +to this change, and the CC line lists people to +notify about the change. +These can be code review nicknames or arbitrary email addresses. +

    + +

    +Replace “<enter description here>” +with a description of your change. +The first line of the change description is conventionally +a one-line summary of the change and is used as the +subject for code review mail; the rest of the +description elaborates. +

    + +

    +The Files 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 math +by deleting the line mentioning regexp.go. +If we did so, the template would now read: +

    + +
    +# Change list.
+# Lines beginning with # are ignored.
+# Multi-line values should be indented.
+
+Reviewer: r, rsc
+CC: math-nuts@swtch.com
+
+Description:
+	Sin, Cos, Tan: improved 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
+
    + +

    +The special sentence “Fixes issue 159.” associates +the change with issue 159 in the Go issue tracker. +When this change is eventually submitted, the issue +tracker will automatically mark the issue as fixed. +

    + +

    +Save the file and exit the editor.

    + +

    +The code review server assigns your change an issue number and URL, +which hg change will print, something like: +

    + +
    +CL created: http://codereview.appspot.com/99999
+
    + +

    +If you need to re-edit the change description, +run hg change 99999. +

    + +

    +You can see a list of your pending changes by running hg pending (hg p for short). +

    + + +

    Synchronize your client

    + +

    While you were working, others might have submitted changes +to the repository. To update your client, run

    + +
    +$ hg sync
+
    + +

    (For Mercurial fans, hg sync runs hg pull -u +but then also synchronizes the local change list state against the new data.)

    + +

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

    + +
    +TODO(rsc): add example of merge
+
    + +

    Mail the change for review

    + +

    To send out a change for review, run hg mail using the change list number +assigned during hg change:

    + +
    +$ hg mail 99999
+
    + +

    You can add to the Reviewer: and CC: lines +using the -ror --cc options. +The above example could have left the Reviewer and CC +lines blank and then run: +

    + +
    +$ hg mail -r r,rsc --cc math-nuts@swtch.com 99999
+
    + +

    to achieve the same effect.

    + +

    Note that -r and --cc cannot be spelled --r or -cc.

    + + +

    Reviewing code

    + +

    +Running hg mail will send an email to you and the reviewers +asking them to visit the issue's URL and make coments on the change. +When done, the reviewer clicks “Publish and Mail comments” +to send comments back. +

    + + +

    Revise and upload

    + +

    You will probably revise your code in response to the reviewer comments. +When you have revised the code and are ready for another round of review, run +

    + +
    +$ hg upload 99999
+
    + +

    to upload the latest copy. +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. +A common acronym in such mails is PTAL: please take another look. +

    +

    +The reviewer can comment on the new copy, and the process repeats. +The reviewer approves the change by replying with a mail that says +LGTM: looks good to me. +

    + +

    Submit the change

    + +

    +Once the code has been LGTM'ed, it is time to submit +it to the Mercurial repository. +If you are a committer, you can run: +

    + +
    +$ hg submit 99999
+
    + +

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

    + +

    +If your local copy of the repository is out of date, +hg submit +will refuse the change: +

    + +
    +$ hg submit 12345678
+local repository out of date; must sync before submit
+
    + +

    +If you are not a committer, you cannot submit the change directly. +Instead, a committer, usually the reviewer who said LGTM, +will run: +

    + +
    +$ hg clpatch 99999
+$ hg submit 99999
+
    + +

    The clpatch command imports your change 99999 into +the committer's local Mercurial client, at which point the committer +can check or test the code more. +(Anyone can run clpatch to try a change that +has been uploaded to the code review server.) +The submit command submits the code. You will be listed as the +author, but the change message will also indicate who the committer was. +

    + + + + +

    The standard copyright header for files in the Go tree is:

    + +
    +// Copyright 2009 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.
+
    + +

    +Code you contribute should have this header. +You need to be listed in the +CONTRIBUTORS file, +which defines who the Go contributors—the people—are; +and the copyright holder for the code you submit (either you or the +organization you work for) needs to be listed in the +AUTHORS file, which defines +who “The Go Authors”—the copyright holders—are. +

    + +

    +When sending your first change list, you should prepare +and send a separate change list adding yourself to +CONTRIBUTORS and adding +the copyright holder for your code to AUTHORS if not already listed. +If you are the copyright holder, you will need to agree to +the individual contributor license agreement, +which can be completed online; +if your organization is the copyright holder, the organization +will need to agree to the corporate contributor license agreement. +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. +One of the Go developers at Google will approve and submit +this change after checking the list of people/organizations +that have completed the agreement. +

    -
  8. Once all.bash succeeds (output like - "N known bugs; 0 unexpected bugs" is OK), - submit a CL. -