| Jamie Hannaford | d5a1cb7 | 2014-10-07 14:31:27 +0200 | [diff] [blame] | 1 | # Contributing to gophercloud |
| 2 | |
| Jamie Hannaford | d2b6dfc | 2014-10-07 14:46:55 +0200 | [diff] [blame] | 3 | - [Getting started](#getting-started) |
| 4 | - [Tests](#tests) |
| 5 | - [Style guide](#basic-style-guide) |
| 6 | - [4 ways to get involved](#4-ways-to-get-involved) |
| 7 | |
| Jamie Hannaford | d2773f2 | 2014-10-07 16:06:23 +0200 | [diff] [blame^] | 8 | ## Setting up your git workspace |
| Jamie Hannaford | d5a1cb7 | 2014-10-07 14:31:27 +0200 | [diff] [blame] | 9 | |
| Jamie Hannaford | d2773f2 | 2014-10-07 16:06:23 +0200 | [diff] [blame^] | 10 | As a contributor you will need to setup your workspace in a slightly different |
| 11 | way than just downloading it. Here are the basic installation instructions: |
| 12 | |
| 13 | 1. clone the upstream repository into a directory of your choice: |
| 14 | |
| 15 | ```bash |
| 16 | mkdir ~/projects/gophercloud && cd ~/projects/gophercloud |
| 17 | export GOPATH=$(pwd) |
| 18 | ``` |
| 19 | |
| 20 | 2. Move into the directory that houses your local repository: |
| 21 | |
| 22 | ```bash |
| 23 | cd src/github.com/rackspace/gophercloud |
| 24 | ``` |
| 25 | |
| 26 | 3. Fork the `rackspace/gophercloud` repository and update your remote refs. You |
| 27 | will need to rename the `origin` remote branch to `upstream`, and add your |
| 28 | fork as `origin` instead: |
| 29 | |
| 30 | ```bash |
| 31 | git remote rename origin upstream |
| 32 | git remote add origin git@github.com/<my_username>/gophercloud |
| 33 | ``` |
| 34 | |
| 35 | 4. Checkout the latest development branch ([click here](/branches) to see all |
| 36 | the branches): |
| 37 | |
| 38 | ```bash |
| 39 | git checkout v0.2.0 |
| 40 | ``` |
| 41 | |
| 42 | 5. If you're working on something (discussed more in detail below), you will |
| 43 | need to checkout a new feature branch: |
| 44 | |
| 45 | ```bash |
| 46 | git checkout -b my-new-feature |
| 47 | ``` |
| 48 | |
| 49 | Another thing to bear in mind is that you will need to add a few extra |
| 50 | environment variables for acceptance tests - this is documented in our |
| 51 | [acceptance tests readme](/acceptance). |
| Jamie Hannaford | d5a1cb7 | 2014-10-07 14:31:27 +0200 | [diff] [blame] | 52 | |
| 53 | ## Tests |
| 54 | |
| 55 | When working on a new or existing feature, testing will be the backbone of your |
| 56 | work since it helps uncover and prevent regressions in the codebase. There are |
| 57 | two types of test we use in gophercloud: unit tests and acceptance tests, which |
| 58 | are both described below. |
| 59 | |
| 60 | ### Unit tests |
| 61 | |
| 62 | Unit tests are the fine-grained tests that establish and ensure the behaviour |
| 63 | of individual units of functionality. We usually test on an |
| 64 | operation-by-operation basis (an operation typically being an API action) with |
| 65 | the use of mocking to set up explicit expectations. Each operation will set up |
| 66 | its HTTP response expectation, and then test how the system responds when fed |
| 67 | this controlled, pre-determined input. |
| 68 | |
| 69 | To make life easier, we've introduced a bunch of test helpers to simplify the |
| 70 | process of testing expectations with assertions: |
| 71 | |
| 72 | ```go |
| 73 | import ( |
| 74 | "testing" |
| 75 | |
| 76 | "github.com/rackspace/gophercloud/testhelper" |
| 77 | ) |
| 78 | |
| Jamie Hannaford | d5a1cb7 | 2014-10-07 14:31:27 +0200 | [diff] [blame] | 79 | func TestSomething(t *testing.T) { |
| Jamie Hannaford | d5a1cb7 | 2014-10-07 14:31:27 +0200 | [diff] [blame] | 80 | result, err := Operation() |
| 81 | |
| 82 | testhelper.AssertEquals(t, "foo", result.Bar) |
| 83 | testhelper.AssertNoErr(t, err) |
| 84 | } |
| Jamie Hannaford | d2b6dfc | 2014-10-07 14:46:55 +0200 | [diff] [blame] | 85 | |
| 86 | func TestSomethingElse(t *testing.T) { |
| 87 | testhelper.CheckEquals(t, "expected", "actual") |
| 88 | } |
| Jamie Hannaford | d5a1cb7 | 2014-10-07 14:31:27 +0200 | [diff] [blame] | 89 | ``` |
| 90 | |
| 91 | `AssertEquals` and `AssertNoErr` will throw a fatal error if a value does not |
| 92 | match an expected value or if an error has been declared, respectively. You can |
| 93 | also use `CheckEquals` and `CheckNoErr` for the same purpose; the only difference |
| 94 | being that `t.Errorf` is raised rather than `t.Fatalf`. |
| 95 | |
| 96 | Here is a truncated example of mocked HTTP responses: |
| 97 | |
| Jamie Hannaford | d2b6dfc | 2014-10-07 14:46:55 +0200 | [diff] [blame] | 98 | ```go |
| Jamie Hannaford | d5a1cb7 | 2014-10-07 14:31:27 +0200 | [diff] [blame] | 99 | import ( |
| 100 | "testing" |
| 101 | |
| 102 | th "github.com/rackspace/gophercloud/testhelper" |
| 103 | fake "github.com/rackspace/gophercloud/testhelper/client" |
| 104 | ) |
| 105 | |
| 106 | func TestGet(t *testing.T) { |
| Jamie Hannaford | d2b6dfc | 2014-10-07 14:46:55 +0200 | [diff] [blame] | 107 | // Setup the HTTP request multiplexer and server |
| Jamie Hannaford | d5a1cb7 | 2014-10-07 14:31:27 +0200 | [diff] [blame] | 108 | th.SetupHTTP() |
| 109 | defer th.TeardownHTTP() |
| 110 | |
| 111 | th.Mux.HandleFunc("/networks/d32019d3-bc6e-4319-9c1d-6722fc136a22", func(w http.ResponseWriter, r *http.Request) { |
| 112 | // Test we're using the correct HTTP method |
| Jamie Hannaford | d2b6dfc | 2014-10-07 14:46:55 +0200 | [diff] [blame] | 113 | th.TestMethod(t, r, "GET") |
| Jamie Hannaford | d5a1cb7 | 2014-10-07 14:31:27 +0200 | [diff] [blame] | 114 | |
| Jamie Hannaford | d2b6dfc | 2014-10-07 14:46:55 +0200 | [diff] [blame] | 115 | // Test we're setting the auth token |
| Jamie Hannaford | d5a1cb7 | 2014-10-07 14:31:27 +0200 | [diff] [blame] | 116 | th.TestHeader(t, r, "X-Auth-Token", fake.TokenID) |
| 117 | |
| Jamie Hannaford | d2b6dfc | 2014-10-07 14:46:55 +0200 | [diff] [blame] | 118 | // Set the appropriate headers for our mocked response |
| Jamie Hannaford | d5a1cb7 | 2014-10-07 14:31:27 +0200 | [diff] [blame] | 119 | w.Header().Add("Content-Type", "application/json") |
| 120 | w.WriteHeader(http.StatusOK) |
| 121 | |
| Jamie Hannaford | d2b6dfc | 2014-10-07 14:46:55 +0200 | [diff] [blame] | 122 | // Set the HTTP body |
| Jamie Hannaford | d5a1cb7 | 2014-10-07 14:31:27 +0200 | [diff] [blame] | 123 | fmt.Fprintf(w, ` |
| 124 | { |
| 125 | "network": { |
| 126 | "status": "ACTIVE", |
| 127 | "name": "private-network", |
| 128 | "admin_state_up": true, |
| 129 | "tenant_id": "4fd44f30292945e481c7b8a0c8908869", |
| 130 | "shared": true, |
| 131 | "id": "d32019d3-bc6e-4319-9c1d-6722fc136a22" |
| 132 | } |
| 133 | } |
| 134 | `) |
| 135 | }) |
| 136 | |
| Jamie Hannaford | d2b6dfc | 2014-10-07 14:46:55 +0200 | [diff] [blame] | 137 | // Call our API operation |
| Jamie Hannaford | d5a1cb7 | 2014-10-07 14:31:27 +0200 | [diff] [blame] | 138 | network, err := Get(fake.ServiceClient(), "d32019d3-bc6e-4319-9c1d-6722fc136a22").Extract() |
| 139 | |
| Jamie Hannaford | d2b6dfc | 2014-10-07 14:46:55 +0200 | [diff] [blame] | 140 | // Assert no errors and equality |
| Jamie Hannaford | d5a1cb7 | 2014-10-07 14:31:27 +0200 | [diff] [blame] | 141 | th.AssertNoErr(t, err) |
| 142 | th.AssertEquals(t, n.Status, "ACTIVE") |
| 143 | } |
| 144 | ``` |
| 145 | |
| Jamie Hannaford | d2b6dfc | 2014-10-07 14:46:55 +0200 | [diff] [blame] | 146 | ### Acceptance tests |
| Jamie Hannaford | d5a1cb7 | 2014-10-07 14:31:27 +0200 | [diff] [blame] | 147 | |
| 148 | As we've already mentioned, unit tests have a very narrow and confined focus - |
| 149 | they test small units of behaviour. Acceptance tests on the other hand have a |
| 150 | far larger scope: they are fully functional tests that test the entire API of a |
| Jamie Hannaford | d2b6dfc | 2014-10-07 14:46:55 +0200 | [diff] [blame] | 151 | service in one fell swoop. They don't care about unit isolation or mocking |
| Jamie Hannaford | d5a1cb7 | 2014-10-07 14:31:27 +0200 | [diff] [blame] | 152 | expectations, they instead do a full run-through and consequently test how the |
| 153 | entire system _integrates_ together. When an API satisfies expectations, it |
| 154 | proves by default that the requirements for a contract have been met. |
| 155 | |
| Jamie Hannaford | d2773f2 | 2014-10-07 16:06:23 +0200 | [diff] [blame^] | 156 | Please be aware that acceptance tests will hit a live API - and may incur |
| 157 | service charges from your provider. Although most tests handle their own |
| 158 | teardown procedures, it is always worth manually checking that resources are |
| 159 | deleted after the test suite finishes. |
| 160 | |
| Jamie Hannaford | d5a1cb7 | 2014-10-07 14:31:27 +0200 | [diff] [blame] | 161 | ### Running tests |
| 162 | |
| 163 | To run all tests: |
| 164 | |
| 165 | ```bash |
| 166 | go test ./... |
| 167 | ``` |
| 168 | |
| 169 | To run all tests with verbose output: |
| 170 | |
| 171 | ```bash |
| 172 | go test -v ./... |
| 173 | ``` |
| 174 | |
| 175 | To run tests that match certain [build tags](): |
| 176 | |
| 177 | ```bash |
| 178 | go test -tags "foo bar" ./... |
| 179 | ``` |
| 180 | |
| 181 | To run tests for a particular sub-package: |
| 182 | |
| 183 | ```bash |
| 184 | cd ./path/to/package && go test . |
| 185 | ``` |
| 186 | |
| 187 | ## Basic style guide |
| 188 | |
| 189 | We follow the standard formatting recommendations and language idioms set out |
| 190 | in the [Effective Go](https://golang.org/doc/effective_go.html) guide. It's |
| 191 | definitely worth reading - but the relevant sections are |
| 192 | [formatting](https://golang.org/doc/effective_go.html#formatting) |
| 193 | and [names](https://golang.org/doc/effective_go.html#names). |
| 194 | |
| Jamie Hannaford | d2773f2 | 2014-10-07 16:06:23 +0200 | [diff] [blame^] | 195 | ## 5 ways to get involved |
| Jamie Hannaford | d5a1cb7 | 2014-10-07 14:31:27 +0200 | [diff] [blame] | 196 | |
| Jamie Hannaford | d2773f2 | 2014-10-07 16:06:23 +0200 | [diff] [blame^] | 197 | There are five main ways you can get involved in our open-source project, and |
| Jamie Hannaford | d5a1cb7 | 2014-10-07 14:31:27 +0200 | [diff] [blame] | 198 | each is described briefly below. Once you've made up your mind and decided on |
| 199 | your fix, you will need to follow the same basic steps that all submissions are |
| 200 | required to adhere to: |
| 201 | |
| 202 | 1. [fork](https://help.github.com/articles/fork-a-repo/) the `rackspace/gophercloud` repository |
| 203 | 2. checkout a [new branch](https://github.com/Kunena/Kunena-Forum/wiki/Create-a-new-branch-with-git-and-manage-branches) |
| Jamie Hannaford | d2773f2 | 2014-10-07 16:06:23 +0200 | [diff] [blame^] | 204 | 3. submit your branch as a [pull request](https://help.github.com/articles/creating-a-pull-request/) |
| Jamie Hannaford | d5a1cb7 | 2014-10-07 14:31:27 +0200 | [diff] [blame] | 205 | |
| Jamie Hannaford | d2773f2 | 2014-10-07 16:06:23 +0200 | [diff] [blame^] | 206 | ### 1. Providing feedback |
| 207 | |
| 208 | On of the easiest ways to get readily involved in our project is to let us know |
| 209 | about your experiences using our SDK. Feedback like this is incredibly useful |
| 210 | to us, because it allows us to refine and change features based on what our |
| 211 | users want and expect of us. There are a bunch of ways to get in contact! You |
| 212 | can [ping us](mailto:sdk-support@rackspace.com) via e-mail, talk to us on irc |
| 213 | (#rackspace-dev on freenode), [tweet us](https://twitter.com/rackspace), or |
| 214 | submit an issue on our [bug tracker](/issues). Things you might like to tell us |
| 215 | are: |
| 216 | |
| 217 | * how easy was it to start using our SDK? |
| 218 | * did it meet your expectations? If not, why not? |
| 219 | * did our documentation help or hinder you? |
| 220 | * what could we improve in general? |
| 221 | |
| 222 | ### 2. Fixing bugs |
| Jamie Hannaford | d5a1cb7 | 2014-10-07 14:31:27 +0200 | [diff] [blame] | 223 | |
| 224 | If you want to start fixing open bugs, we'd really appreciate that! Bug fixing |
| 225 | is central to any project. The best way to get started is by heading to our |
| 226 | [bug tracker](https://github.com/rackspace/gophercloud/issues) and finding open |
| 227 | bugs that you think nobody is working on. It might be useful to comment on the |
| 228 | thread to see the current state of the issue and if anybody has made any |
| 229 | breakthroughs on it so far. |
| 230 | |
| Jamie Hannaford | d2773f2 | 2014-10-07 16:06:23 +0200 | [diff] [blame^] | 231 | ### 3. Improving documentation |
| Jamie Hannaford | d5a1cb7 | 2014-10-07 14:31:27 +0200 | [diff] [blame] | 232 | |
| 233 | We have three forms of documentation: |
| 234 | |
| 235 | * short README documents that briefly introduce a topic |
| Jamie Hannaford | d2b6dfc | 2014-10-07 14:46:55 +0200 | [diff] [blame] | 236 | * reference documentation on [godoc.org](http://godoc.org) that is automatically |
| 237 | generated from source code comments |
| Jamie Hannaford | d5a1cb7 | 2014-10-07 14:31:27 +0200 | [diff] [blame] | 238 | * user documentation on our [homepage](http://gophercloud.io) that includes |
| 239 | getting started guides, installation guides and code samples |
| 240 | |
| 241 | If you feel that a certain section could be improved - whether its to clarify |
| Jamie Hannaford | d2b6dfc | 2014-10-07 14:46:55 +0200 | [diff] [blame] | 242 | ambiguity or fix a grammatical mistake - please feel entitled to do so! We |
| Jamie Hannaford | d5a1cb7 | 2014-10-07 14:31:27 +0200 | [diff] [blame] | 243 | welcome doc pull requests with the same childlike enthusiasm as any other |
| 244 | contribution! |
| 245 | |
| Jamie Hannaford | d2773f2 | 2014-10-07 16:06:23 +0200 | [diff] [blame^] | 246 | ### 4. Optimizing existing features |
| Jamie Hannaford | d5a1cb7 | 2014-10-07 14:31:27 +0200 | [diff] [blame] | 247 | |
| 248 | If you would like to improve or optimize an existing feature, please be aware |
| Jamie Hannaford | d2b6dfc | 2014-10-07 14:46:55 +0200 | [diff] [blame] | 249 | that we adhere to [semantic versioning](http://semver.org) - which means that |
| 250 | we cannot introduce breaking changes to the API without a major version change |
| 251 | (v1.x -> v2.x). Making that leap is a big step, so we encourage contributors to |
| 252 | refactor rather than rewrite. Running tests will prevent regression and avoid |
| 253 | the possibility of breaking somebody's current implementation. |
| Jamie Hannaford | d5a1cb7 | 2014-10-07 14:31:27 +0200 | [diff] [blame] | 254 | |
| Jamie Hannaford | d2773f2 | 2014-10-07 16:06:23 +0200 | [diff] [blame^] | 255 | Another tip is to keep the focus of your work as small as possible - try not to |
| 256 | introduce a change that affects lots and lots of files because it introduces |
| 257 | added risk and increases the cognitive load on the reviewers checking your |
| 258 | work. Change-sets which are easily understood and will not negatively impact |
| 259 | users are more likely to be integrated quickly. |
| 260 | |
| 261 | Lastly, if you're seeking to optimize a particular operation, you should try to |
| 262 | demonstrate a negative performance impact - perhaps using go's inbuilt |
| 263 | [benchmark capabilities](http://dave.cheney.net/2013/06/30/how-to-write-benchmarks-in-go). |
| 264 | |
| 265 | ### 5. Working on a new feature |
| Jamie Hannaford | d5a1cb7 | 2014-10-07 14:31:27 +0200 | [diff] [blame] | 266 | |
| 267 | If you've found something we've left out, definitely feel free to start work on |
| Jamie Hannaford | d2773f2 | 2014-10-07 16:06:23 +0200 | [diff] [blame^] | 268 | introducing that feature. It's always useful to open an issue or submit a pull |
| 269 | request early on to indicate your intent to a core contributor - this enables |
| 270 | quick/early feedback and can help steer you in the right direction by avoiding |
| 271 | known issues. It might also help you avoid losing time implementing something |
| 272 | that might not ever work. One tip is to prefix your Pull Request issue title |
| 273 | with [wip] - then people know it's a work in progress. |
| Jamie Hannaford | d5a1cb7 | 2014-10-07 14:31:27 +0200 | [diff] [blame] | 274 | |
| 275 | You must ensure that all of your work is well tested - both in terms of unit |
| 276 | and acceptance tests. Untested code will not be merged because it introduces |
| Jamie Hannaford | d2b6dfc | 2014-10-07 14:46:55 +0200 | [diff] [blame] | 277 | too much of a risk to end-users. |
| 278 | |
| 279 | Happy hacking! |