|
| 1 | +# Contribution guidelines |
| 2 | + |
| 3 | +## Table of contents |
| 4 | + |
| 5 | +* [Contributing](#contributing) |
| 6 | +* [Writing proper commits - short version](#writing-proper-commits-short-version) |
| 7 | +* [Writing proper commits - long version](#writing-proper-commits-long-version) |
| 8 | +* [Dependencies](#dependencies) |
| 9 | + * [Note for OS X users](#note-for-os-x-users) |
| 10 | +* [The test matrix](#the-test-matrix) |
| 11 | +* [Syntax and style](#syntax-and-style) |
| 12 | +* [Running the unit tests](#running-the-unit-tests) |
| 13 | +* [Unit tests in docker](#unit-tests-in-docker) |
| 14 | +* [Integration tests](#integration-tests) |
| 15 | + |
| 16 | +This module has grown over time based on a range of contributions from |
| 17 | +people using it. If you follow these contributing guidelines your patch |
| 18 | +will likely make it into a release a little more quickly. |
| 19 | + |
| 20 | +## Contributing |
| 21 | + |
| 22 | +Please note that this project is released with a Contributor Code of Conduct. |
| 23 | +By participating in this project you agree to abide by its terms. |
| 24 | +[Contributor Code of Conduct](https://voxpupuli.org/coc/). |
| 25 | + |
| 26 | +* Fork the repo. |
| 27 | +* Create a separate branch for your change. |
| 28 | +* We only take pull requests with passing tests, and documentation. [GitHub Actions](https://docs.github.com/en/actions) run the tests for us. You can also execute them locally. This is explained [in a later section](#the-test-matrix). |
| 29 | +* Checkout [our docs](https://voxpupuli.org/docs/reviewing_pr/) we use to review a module and the [official styleguide](https://puppet.com/docs/puppet/6.0/style_guide.html). They provide some guidance for new code that might help you before you submit a pull request. |
| 30 | +* Add a test for your change. Only refactoring and documentation changes require no new tests. If you are adding functionality or fixing a bug, please add a test. |
| 31 | +* Squash your commits down into logical components. Make sure to rebase against our current master. |
| 32 | +* Push the branch to your fork and submit a pull request. |
| 33 | + |
| 34 | +Please be prepared to repeat some of these steps as our contributors review your code. |
| 35 | + |
| 36 | +Also consider sending in your profile code that calls this component module as an acceptance test or provide it via an issue. This helps reviewers a lot to test your use case and prevents future regressions! |
| 37 | + |
| 38 | +## Writing proper commits - short version |
| 39 | + |
| 40 | +* Make commits of logical units. |
| 41 | +* Check for unnecessary whitespace with "git diff --check" before committing. |
| 42 | +* Commit using Unix line endings (check the settings around "crlf" in git-config(1)). |
| 43 | +* Do not check in commented out code or unneeded files. |
| 44 | +* The first line of the commit message should be a short description (50 characters is the soft limit, excluding ticket number(s)), and should skip the full stop. |
| 45 | +* Associate the issue in the message. The first line should include the issue number in the form "(#XXXX) Rest of message". |
| 46 | +* The body should provide a meaningful commit message, which: |
| 47 | + *uses the imperative, present tense: `change`, not `changed` or `changes`. |
| 48 | + * includes motivation for the change, and contrasts its implementation with the previous behavior. |
| 49 | + * Make sure that you have tests for the bug you are fixing, or feature you are adding. |
| 50 | + * Make sure the test suites passes after your commit: |
| 51 | + * When introducing a new feature, make sure it is properly documented in the README.md |
| 52 | + |
| 53 | +## Writing proper commits - long version |
| 54 | + |
| 55 | + 1. Make separate commits for logically separate changes. |
| 56 | + |
| 57 | + Please break your commits down into logically consistent units |
| 58 | + which include new or changed tests relevant to the rest of the |
| 59 | + change. The goal of doing this is to make the diff easier to |
| 60 | + read for whoever is reviewing your code. In general, the easier |
| 61 | + your diff is to read, the more likely someone will be happy to |
| 62 | + review it and get it into the code base. |
| 63 | + |
| 64 | + If you are going to refactor a piece of code, please do so as a |
| 65 | + separate commit from your feature or bug fix changes. |
| 66 | + |
| 67 | + We also really appreciate changes that include tests to make |
| 68 | + sure the bug is not re-introduced, and that the feature is not |
| 69 | + accidentally broken. |
| 70 | + |
| 71 | + Describe the technical detail of the change(s). If your |
| 72 | + description starts to get too long, that is a good sign that you |
| 73 | + probably need to split up your commit into more finely grained |
| 74 | + pieces. |
| 75 | + |
| 76 | + Commits which plainly describe the things which help |
| 77 | + reviewers check the patch and future developers understand the |
| 78 | + code are much more likely to be merged in with a minimum of |
| 79 | + bike-shedding or requested changes. Ideally, the commit message |
| 80 | + would include information, and be in a form suitable for |
| 81 | + inclusion in the release notes for the version of Puppet that |
| 82 | + includes them. |
| 83 | + |
| 84 | + Please also check that you are not introducing any trailing |
| 85 | + whitespace or other "whitespace errors". You can do this by |
| 86 | + running "git diff --check" on your changes before you commit. |
| 87 | + |
| 88 | + 2. Sending your patches |
| 89 | + |
| 90 | + To submit your changes via a GitHub pull request, we _highly_ |
| 91 | + recommend that you have them on a topic branch, instead of |
| 92 | + directly on `master`. |
| 93 | + It makes things much easier to keep track of, especially if |
| 94 | + you decide to work on another thing before your first change |
| 95 | + is merged in. |
| 96 | + |
| 97 | + GitHub has some pretty good |
| 98 | + [general documentation](http://help.github.com/) on using |
| 99 | + their site. They also have documentation on |
| 100 | + [creating pull requests](http://help.github.com/send-pull-requests/). |
| 101 | + |
| 102 | + In general, after pushing your topic branch up to your |
| 103 | + repository on GitHub, you can switch to the branch in the |
| 104 | + GitHub UI and click "Pull Request" towards the top of the page |
| 105 | + in order to open a pull request. |
| 106 | + |
| 107 | + |
| 108 | + 3. Update the related GitHub issue. |
| 109 | + |
| 110 | + If there is a GitHub issue associated with the change you |
| 111 | + submitted, then you should update the ticket to include the |
| 112 | + location of your branch, along with any other commentary you |
| 113 | + may wish to make. |
| 114 | + |
| 115 | +## Dependencies |
| 116 | + |
| 117 | +The testing and development tools have a bunch of dependencies, |
| 118 | +all managed by [bundler](http://bundler.io/) according to the |
| 119 | +[Puppet support matrix](http://docs.puppetlabs.com/guides/platforms.html#ruby-versions). |
| 120 | + |
| 121 | +By default the tests use a baseline version of Puppet. |
| 122 | + |
| 123 | +If you have Ruby 2.x or want a specific version of Puppet, |
| 124 | +you must set an environment variable such as: |
| 125 | + |
| 126 | +```sh |
| 127 | +export PUPPET_VERSION="~> 5.5.6" |
| 128 | +``` |
| 129 | + |
| 130 | +You can install all needed gems for spec tests into the modules directory by |
| 131 | +running: |
| 132 | + |
| 133 | +```sh |
| 134 | +bundle install --path .vendor/ --without development system_tests release --jobs "$(nproc)" |
| 135 | +``` |
| 136 | + |
| 137 | +If you also want to run acceptance tests: |
| 138 | + |
| 139 | +```sh |
| 140 | +bundle install --path .vendor/ --with system_tests --without development release --jobs "$(nproc)" |
| 141 | +``` |
| 142 | + |
| 143 | +Our all in one solution if you don't know if you need to install or update gems: |
| 144 | + |
| 145 | +```sh |
| 146 | +bundle install --path .vendor/ --with system_tests --without development release --jobs "$(nproc)"; bundle update; bundle clean |
| 147 | +``` |
| 148 | + |
| 149 | +As an alternative to the `--jobs "$(nproc)` parameter, you can set an |
| 150 | +environment variable: |
| 151 | + |
| 152 | +```sh |
| 153 | +BUNDLE_JOBS="$(nproc)" |
| 154 | +``` |
| 155 | + |
| 156 | +### Note for OS X users |
| 157 | + |
| 158 | +`nproc` isn't a valid command under OS x. As an alternative, you can do: |
| 159 | + |
| 160 | +```sh |
| 161 | +--jobs "$(sysctl -n hw.ncpu)" |
| 162 | +``` |
| 163 | + |
| 164 | +## The test matrix |
| 165 | + |
| 166 | +### Syntax and style |
| 167 | + |
| 168 | +The test suite will run [Puppet Lint](http://puppet-lint.com/) and |
| 169 | +[Puppet Syntax](https://github.com/gds-operations/puppet-syntax) to |
| 170 | +check various syntax and style things. You can run these locally with: |
| 171 | + |
| 172 | +```sh |
| 173 | +bundle exec rake lint |
| 174 | +bundle exec rake validate |
| 175 | +``` |
| 176 | + |
| 177 | +It will also run some [Rubocop](http://batsov.com/rubocop/) tests |
| 178 | +against it. You can run those locally ahead of time with: |
| 179 | + |
| 180 | +```sh |
| 181 | +bundle exec rake rubocop |
| 182 | +``` |
| 183 | + |
| 184 | +### Running the unit tests |
| 185 | + |
| 186 | +The unit test suite covers most of the code, as mentioned above please |
| 187 | +add tests if you're adding new functionality. If you've not used |
| 188 | +[rspec-puppet](http://rspec-puppet.com/) before then feel free to ask |
| 189 | +about how best to test your new feature. |
| 190 | + |
| 191 | +To run the linter, the syntax checker and the unit tests: |
| 192 | + |
| 193 | +```sh |
| 194 | +bundle exec rake test |
| 195 | +``` |
| 196 | + |
| 197 | +To run your all the unit tests |
| 198 | + |
| 199 | +```sh |
| 200 | +bundle exec rake spec |
| 201 | +``` |
| 202 | + |
| 203 | +To run a specific spec test set the `SPEC` variable: |
| 204 | + |
| 205 | +```sh |
| 206 | +bundle exec rake spec SPEC=spec/foo_spec.rb |
| 207 | +``` |
| 208 | + |
| 209 | +#### Unit tests in docker |
| 210 | + |
| 211 | +Some people don't want to run the dependencies locally or don't want to install |
| 212 | +ruby. We ship a Dockerfile that enables you to run all unit tests and linting. |
| 213 | +You only need to run: |
| 214 | + |
| 215 | +```sh |
| 216 | +docker build . |
| 217 | +``` |
| 218 | + |
| 219 | +Please ensure that a docker daemon is running and that your user has the |
| 220 | +permission to talk to it. You can specify a remote docker host by setting the |
| 221 | +`DOCKER_HOST` environment variable. it will copy the content of the module into |
| 222 | +the docker image. So it will not work if a Gemfile.lock exists. |
| 223 | + |
| 224 | +### Integration tests |
| 225 | + |
| 226 | +The unit tests just check the code runs, not that it does exactly what |
| 227 | +we want on a real machine. For that we're using |
| 228 | +[beaker](https://github.com/puppetlabs/beaker). |
| 229 | + |
| 230 | +This fires up a new virtual machine (using vagrant) and runs a series of |
| 231 | +simple tests against it after applying the module. You can run this |
| 232 | +with: |
| 233 | + |
| 234 | +```sh |
| 235 | +BEAKER_setfile=debian10-x64 bundle exec rake beaker |
| 236 | +``` |
| 237 | + |
| 238 | +You can replace the string `debian10` with any common operating system. |
| 239 | +The following strings are known to work: |
| 240 | + |
| 241 | +* ubuntu1604 |
| 242 | +* ubuntu1804 |
| 243 | +* ubuntu2004 |
| 244 | +* debian9 |
| 245 | +* debian10 |
| 246 | +* centos7 |
| 247 | +* centos8 |
| 248 | + |
| 249 | +For more information and tips & tricks, see [voxpupuli-acceptance's documentation](https://github.com/voxpupuli/voxpupuli-acceptance#running-tests). |
| 250 | + |
| 251 | +The source of this file is in our [modulesync_config](https://github.com/voxpupuli/modulesync_config/blob/master/moduleroot/.github/CONTRIBUTING.md.erb) |
| 252 | +repository. |
0 commit comments