|
| 1 | +Checklist (and a short version for the impatient) |
| 2 | +================================================= |
| 3 | + |
| 4 | + * Commits: |
| 5 | + |
| 6 | + - Make commits of logical units. |
| 7 | + |
| 8 | + - Check for unnecessary whitespace with "git diff --check" before |
| 9 | + committing. |
| 10 | + |
| 11 | + - Commit using Unix line endings (check the settings around "crlf" in |
| 12 | + git-config(1)). |
| 13 | + |
| 14 | + - Do not check in commented out code or unneeded files. |
| 15 | + |
| 16 | + - The first line of the commit message should be a short |
| 17 | + description (50 characters is the soft limit, excluding ticket |
| 18 | + number(s)), and should skip the full stop. |
| 19 | + |
| 20 | + - Associate the issue in the message. The first line should include |
| 21 | + the issue number in the form "(#XXXX) Rest of message". |
| 22 | + |
| 23 | + - The body should provide a meaningful commit message, which: |
| 24 | + |
| 25 | + - uses the imperative, present tense: "change", not "changed" or |
| 26 | + "changes". |
| 27 | + |
| 28 | + - includes motivation for the change, and contrasts its |
| 29 | + implementation with the previous behavior. |
| 30 | + |
| 31 | + - Make sure that you have tests for the bug you are fixing, or |
| 32 | + feature you are adding. |
| 33 | + |
| 34 | + - Make sure the test suites passes after your commit: |
| 35 | + `bundle exec rspec spec/acceptance` More information on [testing](#Testing) below |
| 36 | + |
| 37 | + - When introducing a new feature, make sure it is properly |
| 38 | + documented in the README.md |
| 39 | + |
| 40 | + * Submission: |
| 41 | + |
| 42 | + * Pre-requisites: |
| 43 | + |
| 44 | + - Make sure you have a [GitHub account](https://github.com/join) |
| 45 | + |
| 46 | + - [Create a ticket](https://tickets.puppetlabs.com/secure/CreateIssue!default.jspa), or [watch the ticket](https://tickets.puppetlabs.com/browse/) you are patching for. |
| 47 | + |
| 48 | + * Preferred method: |
| 49 | + |
| 50 | + - Fork the repository on GitHub. |
| 51 | + |
| 52 | + - Push your changes to a topic branch in your fork of the |
| 53 | + repository. (the format ticket/1234-short_description_of_change is |
| 54 | + usually preferred for this project). |
| 55 | + |
| 56 | + - Submit a pull request to the repository in the puppetlabs |
| 57 | + organization. |
| 58 | + |
| 59 | +The long version |
| 60 | +================ |
| 61 | + |
| 62 | + 1. Make separate commits for logically separate changes. |
| 63 | + |
| 64 | + Please break your commits down into logically consistent units |
| 65 | + which include new or changed tests relevant to the rest of the |
| 66 | + change. The goal of doing this is to make the diff easier to |
| 67 | + read for whoever is reviewing your code. In general, the easier |
| 68 | + your diff is to read, the more likely someone will be happy to |
| 69 | + review it and get it into the code base. |
| 70 | + |
| 71 | + If you are going to refactor a piece of code, please do so as a |
| 72 | + separate commit from your feature or bug fix changes. |
| 73 | + |
| 74 | + We also really appreciate changes that include tests to make |
| 75 | + sure the bug is not re-introduced, and that the feature is not |
| 76 | + accidentally broken. |
| 77 | + |
| 78 | + Describe the technical detail of the change(s). If your |
| 79 | + description starts to get too long, that is a good sign that you |
| 80 | + probably need to split up your commit into more finely grained |
| 81 | + pieces. |
| 82 | + |
| 83 | + Commits which plainly describe the things which help |
| 84 | + reviewers check the patch and future developers understand the |
| 85 | + code are much more likely to be merged in with a minimum of |
| 86 | + bike-shedding or requested changes. Ideally, the commit message |
| 87 | + would include information, and be in a form suitable for |
| 88 | + inclusion in the release notes for the version of Puppet that |
| 89 | + includes them. |
| 90 | + |
| 91 | + Please also check that you are not introducing any trailing |
| 92 | + whitespace or other "whitespace errors". You can do this by |
| 93 | + running "git diff --check" on your changes before you commit. |
| 94 | + |
| 95 | + 2. Sending your patches |
| 96 | + |
| 97 | + To submit your changes via a GitHub pull request, we _highly_ |
| 98 | + recommend that you have them on a topic branch, instead of |
| 99 | + directly on "master". |
| 100 | + It makes things much easier to keep track of, especially if |
| 101 | + you decide to work on another thing before your first change |
| 102 | + is merged in. |
| 103 | + |
| 104 | + GitHub has some pretty good |
| 105 | + [general documentation](http://help.github.com/) on using |
| 106 | + their site. They also have documentation on |
| 107 | + [creating pull requests](http://help.github.com/send-pull-requests/). |
| 108 | + |
| 109 | + In general, after pushing your topic branch up to your |
| 110 | + repository on GitHub, you can switch to the branch in the |
| 111 | + GitHub UI and click "Pull Request" towards the top of the page |
| 112 | + in order to open a pull request. |
| 113 | + |
| 114 | + |
| 115 | + 3. Update the related GitHub issue. |
| 116 | + |
| 117 | + If there is a GitHub issue associated with the change you |
| 118 | + submitted, then you should update the ticket to include the |
| 119 | + location of your branch, along with any other commentary you |
| 120 | + may wish to make. |
| 121 | + |
| 122 | +Testing |
| 123 | +======= |
| 124 | + |
| 125 | +Getting Started |
| 126 | +--------------- |
| 127 | + |
| 128 | +Our puppet modules provide [`Gemfile`](./Gemfile)s which can tell a ruby |
| 129 | +package manager such as [bundler](http://bundler.io/) what Ruby packages, |
| 130 | +or Gems, are required to build, develop, and test this software. |
| 131 | + |
| 132 | +Please make sure you have [bundler installed](http://bundler.io/#getting-started) |
| 133 | +on your system, then use it to install all dependencies needed for this project, |
| 134 | +by running |
| 135 | + |
| 136 | +```shell |
| 137 | +% bundle install |
| 138 | +Fetching gem metadata from https://rubygems.org/........ |
| 139 | +Fetching gem metadata from https://rubygems.org/.. |
| 140 | +Using rake (10.1.0) |
| 141 | +Using builder (3.2.2) |
| 142 | +-- 8><-- many more --><8 -- |
| 143 | +Using rspec-system-puppet (2.2.0) |
| 144 | +Using serverspec (0.6.3) |
| 145 | +Using rspec-system-serverspec (1.0.0) |
| 146 | +Using bundler (1.3.5) |
| 147 | +Your bundle is complete! |
| 148 | +Use `bundle show [gemname]` to see where a bundled gem is installed. |
| 149 | +``` |
| 150 | + |
| 151 | +NOTE some systems may require you to run this command with sudo. |
| 152 | + |
| 153 | +If you already have those gems installed, make sure they are up-to-date: |
| 154 | + |
| 155 | +```shell |
| 156 | +% bundle update |
| 157 | +``` |
| 158 | + |
| 159 | +With all dependencies in place and up-to-date we can now run the tests: |
| 160 | + |
| 161 | +```shell |
| 162 | +% rake spec |
| 163 | +``` |
| 164 | + |
| 165 | +This will execute all the [rspec tests](http://rspec-puppet.com/) tests |
| 166 | +under [spec/defines](./spec/defines), [spec/classes](./spec/classes), |
| 167 | +and so on. rspec tests may have the same kind of dependencies as the |
| 168 | +module they are testing. While the module defines in its [Modulefile](./Modulefile), |
| 169 | +rspec tests define them in [.fixtures.yml](./fixtures.yml). |
| 170 | + |
| 171 | +Some puppet modules also come with [beaker](https://github.com/puppetlabs/beaker) |
| 172 | +tests. These tests spin up a virtual machine under |
| 173 | +[VirtualBox](https://www.virtualbox.org/)) with, controlling it with |
| 174 | +[Vagrant](http://www.vagrantup.com/) to actually simulate scripted test |
| 175 | +scenarios. In order to run these, you will need both of those tools |
| 176 | +installed on your system. |
| 177 | + |
| 178 | +You can run them by issuing the following command |
| 179 | + |
| 180 | +```shell |
| 181 | +% rake spec_clean |
| 182 | +% rspec spec/acceptance |
| 183 | +``` |
| 184 | + |
| 185 | +This will now download a pre-fabricated image configured in the [default node-set](./spec/acceptance/nodesets/default.yml), |
| 186 | +install puppet, copy this module and install its dependencies per [spec/spec_helper_acceptance.rb](./spec/spec_helper_acceptance.rb) |
| 187 | +and then run all the tests under [spec/acceptance](./spec/acceptance). |
| 188 | + |
| 189 | +Writing Tests |
| 190 | +------------- |
| 191 | + |
| 192 | +XXX getting started writing tests. |
| 193 | + |
| 194 | +If you have commit access to the repository |
| 195 | +=========================================== |
| 196 | + |
| 197 | +Even if you have commit access to the repository, you will still need to |
| 198 | +go through the process above, and have someone else review and merge |
| 199 | +in your changes. The rule is that all changes must be reviewed by a |
| 200 | +developer on the project (that did not write the code) to ensure that |
| 201 | +all changes go through a code review process. |
| 202 | + |
| 203 | +Having someone other than the author of the topic branch recorded as |
| 204 | +performing the merge is the record that they performed the code |
| 205 | +review. |
| 206 | + |
| 207 | + |
| 208 | +Additional Resources |
| 209 | +==================== |
| 210 | + |
| 211 | +* [Getting additional help](http://puppetlabs.com/community/get-help) |
| 212 | + |
| 213 | +* [Writing tests](http://projects.puppetlabs.com/projects/puppet/wiki/Development_Writing_Tests) |
| 214 | + |
| 215 | +* [Patchwork](https://patchwork.puppetlabs.com) |
| 216 | + |
| 217 | +* [General GitHub documentation](http://help.github.com/) |
| 218 | + |
| 219 | +* [GitHub pull request documentation](http://help.github.com/send-pull-requests/) |
0 commit comments