~themue/juju-core/053-env-more-script-friendly

713.3.1 by Dave Cheney
added CONTRIBUTING
1
Getting started
2
===============
3
713.3.4 by Dave Cheney
responding to review feedback
4
Before contributing to `juju-core` please read the following sections describing
5
the tools and conventions of this project. This file is a companion to README
6
and it is assumed that file has been read prior.
713.3.1 by Dave Cheney
added CONTRIBUTING
7
8
cobzr
9
-----
10
713.3.4 by Dave Cheney
responding to review feedback
11
juju-core uses bzr for source control. To make the bzr branching strategy more
12
palatable for the go toolset `juju-core` recommends using the `cobzr` helper.
13
`cobzr` allows the user to juggle multiple branches in place in a way that is
14
compatible with GOPATH. While it is possible to manage bzr branches manually,
15
the remainder of this document will assume cobzr is in use as practiced by the
16
juju-core team.
713.3.1 by Dave Cheney
added CONTRIBUTING
17
911.1.1 by Brad Crittenden
Move guts of get command to statecmd
18
To install `cobzr`,
713.3.1 by Dave Cheney
added CONTRIBUTING
19
20
    go get launchpad.net/cobzr
21
22
TODO(dfc) document PPA version
23
911.1.1 by Brad Crittenden
Move guts of get command to statecmd
24
It is recommended to use alias `cobzr` to `bzr`.
713.3.1 by Dave Cheney
added CONTRIBUTING
25
26
    alias bzr=cobzr
27
713.3.4 by Dave Cheney
responding to review feedback
28
As `cobzr` passes any options it does not handle to `bzr`, it is safe, and recommended
29
to add this alias command to your login shell.
713.3.1 by Dave Cheney
added CONTRIBUTING
30
31
lbox
32
----
33
713.3.4 by Dave Cheney
responding to review feedback
34
`juju-core` uses `lbox` for code review and submission. In addition `lbox` enables
35
the use of prerequisite branches.
713.3.1 by Dave Cheney
added CONTRIBUTING
36
37
To install lbox
38
39
    go get launchpad.net/lbox
40
41
TODO(dfc) document PPA version
42
43
Branching
44
=========
45
713.3.4 by Dave Cheney
responding to review feedback
46
All changes to `juju-core` must be performed on a branch, that branch reviewed,
47
then submitted. An overview of the commands used to do so follows. These
48
examples use the `bzr` command, which is assumed to be aliased to `cobzr`, as
911.1.1 by Brad Crittenden
Move guts of get command to statecmd
49
described above. It is also assumed that your working directory is
826.1.1 by Dave Cheney
fix incorrect src path
50
$GOPATH/src/launchpad.net/juju-core.
713.3.4 by Dave Cheney
responding to review feedback
51
713.3.5 by Dave Cheney
final tweaks
52
First, create a branch for your change using the following command
713.3.1 by Dave Cheney
added CONTRIBUTING
53
54
    bzr branch lp:juju-core/ add-CONTRIBUTING
55
713.3.5 by Dave Cheney
final tweaks
56
This will branch `juju-core` and create a new branch called `add-CONTRIBUTING` in
57
your local workspace. Importantly this will not switch to this branch immediately,
713.3.4 by Dave Cheney
responding to review feedback
58
so to switch to this branch use the following
713.3.1 by Dave Cheney
added CONTRIBUTING
59
60
    bzr switch add-CONTRIBUTING
61
713.3.4 by Dave Cheney
responding to review feedback
62
At this point your previous branch will be stashed and the working copy updated
713.3.5 by Dave Cheney
final tweaks
63
to match the state of the `add-CONTRIBUTING` branch. You must ensure any
64
outstanding changes to the previous branch are committed or reverted to avoid
65
local merge issues.
713.3.1 by Dave Cheney
added CONTRIBUTING
66
67
You can also list any branches you are currently working on by
68
69
    bzr branch
70
1408.1.1 by John Arbash Meinel
Start changing the imports of the middle level files.
71
72
Imports
73
-------
74
75
Import statements are grouped into 3 sections: standard library, 3rd party
76
libraries, juju-core imports. The tool "go fmt" can be used to ensure each
77
group is alphabetically sorted. eg:
78
79
    import (
80
        "fmt"
81
        "time"
82
83
        "labix.org/v2/mgo"
84
        gc "launchpad.net/gocheck"
85
        "launchpad.net/loggo"
86
87
        "launchpad.net/juju-core/state"
88
        "launchpad.net/juju-core/worker"
89
    )
90
91
Because "launchpad.net/gocheck" will be referenced frequently in test suites,
92
its name gets a default short name of just "gc".
93
713.3.1 by Dave Cheney
added CONTRIBUTING
94
Testing
95
=======
96
713.3.4 by Dave Cheney
responding to review feedback
97
`juju-core` uses the `gocheck` testing framework. `gocheck` is automatically
98
installed as a dependency of `juju-core`. You can read more about `gocheck`
99
at http://go.pkgdoc.org/pkg/launchpad.net/gocheck. `gocheck` is integrated
100
into the source of each package so the standard `go test` command is used
101
to run `gocheck` tests. For example
713.3.1 by Dave Cheney
added CONTRIBUTING
102
103
    go test launchpad.net/juju-core/...
104
713.3.4 by Dave Cheney
responding to review feedback
105
will run all the tests in the `juju-core` project. By default `gocheck` prints
106
only minimal output, and as `gocheck` is hooked into the testing framework via
107
a single `go test` test per package, the usual `go test -v` flags are less
108
useful. As a replacement the following commands produce more output from
109
`gocheck`.
713.3.1 by Dave Cheney
added CONTRIBUTING
110
911.1.1 by Brad Crittenden
Move guts of get command to statecmd
111
    go test -gocheck.v
713.3.1 by Dave Cheney
added CONTRIBUTING
112
713.3.4 by Dave Cheney
responding to review feedback
113
is similar to `go test -v` and outputs the name of each test as it is run as
114
well as any logging statements. It is important to note that these statements
115
are buffered until the test completes.
713.3.1 by Dave Cheney
added CONTRIBUTING
116
911.1.1 by Brad Crittenden
Move guts of get command to statecmd
117
    go test -gocheck.vv
713.3.1 by Dave Cheney
added CONTRIBUTING
118
713.3.4 by Dave Cheney
responding to review feedback
119
extends the previous example by outputting any logging data immediately, rather
120
than waiting for the test to complete. By default `gocheck` will run all tests
121
in a package, selected tests can by run by passing `-gocheck.f`
713.3.1 by Dave Cheney
added CONTRIBUTING
122
123
    go test -gocheck.f '$REGEX'
124
125
to match a subset of test names.
126
713.3.4 by Dave Cheney
responding to review feedback
127
Finally, because by default `go test` runs the tests in the current package, and
128
is not recursive, the following commands are equal, and will produce no output.
713.3.1 by Dave Cheney
added CONTRIBUTING
129
826.1.1 by Dave Cheney
fix incorrect src path
130
    cd $GOPATH/src/launchpad.net/juju-core
713.3.1 by Dave Cheney
added CONTRIBUTING
131
    go test
132
133
    go test launchpad.net/juju-core
134
135
Proposing
136
=========
137
911.1.1 by Brad Crittenden
Move guts of get command to statecmd
138
All code review is done on rietveld (http://code.google.com/p/rietveld/), not
911.1.5 by Brad Crittenden
Suggested changes to documentation from review.
139
on launchpad.
713.3.5 by Dave Cheney
final tweaks
140
141
Note: If this is your first time using `lbox` you will also be prompted to visit
142
lauchpad to authorise an oauth token for `lbox`
143
713.3.4 by Dave Cheney
responding to review feedback
144
Once your change is ready, and you have successfully run all the tests you can
145
propose your branch for merging
146
147
    lbox propose
148
713.3.5 by Dave Cheney
final tweaks
149
`lbox` will prompt you for a branch description and will create a rietveld code
150
review for this change, linking it to the launchpad branch, and mailing
151
reviewers. The `lbox` tool manages the state of the launchpad review, with the
152
exception of marking reviews as rejected or work in progress by the reviewer.
153
If you abandon a proposal, you should mark it as rejected in launchpad to avoid
154
wasting the time of others. If you decide to start again, you should add a
155
comment to the abandoned rietveld proposal linking it to your replacement.
713.3.4 by Dave Cheney
responding to review feedback
156
157
If your proposal requires additional work, then you can use the `lbox propose`
158
command again to re-propose, this will also instruct rietveld to mail off any
159
comments to your reviewers and upload fresh diff.
160
161
If your branch requires another branch as a prerequisite use the `-req` flag to
162
indicate so
713.3.1 by Dave Cheney
added CONTRIBUTING
163
164
    lbox propose -req lp:dave-cheney/add-README
165
713.3.4 by Dave Cheney
responding to review feedback
166
This will produce a diff in rietveld which masks the changes from your prereq.
713.3.1 by Dave Cheney
added CONTRIBUTING
167
713.3.4 by Dave Cheney
responding to review feedback
168
It is sometimes useful to be able to review your branch in rietveld without
169
proposing, do to this the `-wip` flag is used
713.3.1 by Dave Cheney
added CONTRIBUTING
170
171
    lbox propose -wip
172
713.3.4 by Dave Cheney
responding to review feedback
173
You can also edit the description of your change with `-edit`. This is useful
174
when combined with the `-wip` flag to avoid sending too many mails to reviewers
175
while preparing your proposal.
713.3.1 by Dave Cheney
added CONTRIBUTING
176
177
    lbox propose -edit
178
713.3.5 by Dave Cheney
final tweaks
179
By default, lbox creates a new bug in launchpad for each proposed branch, and
180
assigns the branch to it. If you wish to assign it to an existing bug instead,
181
use the `-bug` flag to link the branch inside launchpad.
713.3.1 by Dave Cheney
added CONTRIBUTING
182
911.1.1 by Brad Crittenden
Move guts of get command to statecmd
183
    lbox propose -bug 1234567
713.3.1 by Dave Cheney
added CONTRIBUTING
184
713.3.5 by Dave Cheney
final tweaks
185
There is currently a bug with `lbox` when linking a branch to a bug which sets
186
the milestone field to the last milestone defined on the project. Generally
713.3.4 by Dave Cheney
responding to review feedback
187
this is not what you want, so you should visit the bug's page and correct the
188
milestone, if set.
713.3.1 by Dave Cheney
added CONTRIBUTING
189
190
Code review
191
===========
192
713.3.4 by Dave Cheney
responding to review feedback
193
`juju-core` operates on a two positive, no negative review process. You may not
194
submit your proposal until it has received two LGTM comments. If any NOT LGTM
195
comments are received, those comments should be resolved to the satisfaction
196
of the objecting reviewer before submitting. Once your have received at least
1590.2.4 by Roger Peppe
include dependency info in CONTRIBUTING
197
two positive reviews, you can submit your branch by going to the launchpad
198
merge proposal page and:
199
200
    - copy and paste the merge proposal description into the
201
    commit message.
202
    - mark the proposal as Approved.
203
204
The merge proposal will then be tested and merged into trunk assuming
205
all tests pass cleanly.
713.3.1 by Dave Cheney
added CONTRIBUTING
206
207
lbox hooks
208
----------
209
713.3.4 by Dave Cheney
responding to review feedback
210
Before proposing, `lbox` runs a number of hooks to improve code quality and
211
ensure that code is properly formatted. These checks are in
826.1.1 by Dave Cheney
fix incorrect src path
212
`$GOPATH/src/launchpad.net/juju-core/.lbox.check`. They are run automatically
713.3.4 by Dave Cheney
responding to review feedback
213
by `lbox` before proposing or submitting. If these hooks fail you will have
214
to resolve those errors before trying again. For example
713.3.1 by Dave Cheney
added CONTRIBUTING
215
216
    % lbox propose
217
    gofmt is sad:
218
219
        version/version.go
220
1590.2.4 by Roger Peppe
include dependency info in CONTRIBUTING
221
Dependency management
222
=====================
223
224
In the top-level directory, there is a file, dependencies.tsv, that
225
holds the revision ids of all the external projects that juju-core
226
depends on. The tab-separated columns in the file are
227
the project name, the type version control system used by
228
that project, and the revision id and number respectively.
229
230
This file is generated by running the godeps command (which you
231
can get with `go get launchpad.net/godeps') on a juju-core
232
installation with all freshly downloaded directories.
233
234
The bash commands used to generate it from scratch are as follows:
235
236
    % export GOPATH=/tmp/juju-build
237
    % go get launchpad.net/juju-core/...
238
    % go test launchpad.net/juju-core/...
239
    % godeps -t $(go list launchpad.net/juju-core/...) > dependencies.tsv