|
| 1 | +Contributing to UV-CDAT |
| 2 | +====================== |
| 3 | + |
| 4 | +Where to start? |
| 5 | +--------------- |
| 6 | + |
| 7 | +All contributions, bug reports, bug fixes, documentation improvements, |
| 8 | +enhancements and ideas are welcome. |
| 9 | + |
| 10 | +If you are simply looking to start working with the *UV-CDAT* codebase, |
| 11 | +navigate to the [GitHub "issues" |
| 12 | +tab](https://github.com/UV-CDAT/uvcdat/issues) and start looking through |
| 13 | +interesting issues. |
| 14 | + |
| 15 | +Feel free to ask questions on [mailing |
| 16 | +list](uvcdat-users@llnl.gov) or [askbot](http://askbot-uvcdat.llnl.gov/questions) |
| 17 | + |
| 18 | +Bug Reports/Enhancement Requests |
| 19 | +-------------------------------- |
| 20 | + |
| 21 | +Bug reports are an important part of making *UV-CDAT* more stable. Having |
| 22 | +a complete bug report will allow others to reproduce the bug and provide |
| 23 | +insight into fixing. Since many versions of *UV-CDAT* are supported, |
| 24 | +knowing version information will also identify improvements made since |
| 25 | +previous versions. Often trying the bug-producing code out on the |
| 26 | +*master* branch is a worthwhile exercise to confirm the bug still |
| 27 | +exists. It is also worth searching existing bug reports and pull |
| 28 | +requests to see if the issue has already been reported and/or fixed. |
| 29 | + |
| 30 | +Bug reports must: |
| 31 | + |
| 32 | +1. Include a short, self-contained Python snippet reproducing the |
| 33 | + problem. You can have the code formatted nicely by using [GitHub |
| 34 | + Flavored |
| 35 | + Markdown](http://github.github.com/github-flavored-markdown/): : |
| 36 | + |
| 37 | + ```python |
| 38 | + >>> import vcs |
| 39 | + >>> vcs.init() |
| 40 | + ... |
| 41 | + ``` |
| 42 | + |
| 43 | +2. Explain why the current behavior is wrong/not desired and what you |
| 44 | + expect instead. |
| 45 | + |
| 46 | +The issue will then show up to the *UV-CDAT* community and be open to |
| 47 | +comments/ideas from others. |
| 48 | + |
| 49 | +Working with the code |
| 50 | +--------------------- |
| 51 | + |
| 52 | +Now that you have an issue you want to fix, enhancement to add, or |
| 53 | +documentation to improve, you need to learn how to work with GitHub and |
| 54 | +the *UV-CDAT* code base. |
| 55 | + |
| 56 | +### Version Control, Git, and GitHub |
| 57 | + |
| 58 | +To the new user, working with Git is one of the more daunting aspects of |
| 59 | +contributing to *UV-CDAT*. It can very quickly become overwhelming, but |
| 60 | +sticking to the guidelines below will make the process straightforward |
| 61 | +and will work without much trouble. As always, if you are having |
| 62 | +difficulties please feel free to ask for help. |
| 63 | + |
| 64 | +The code is hosted on [GitHub](https://www.github.com/UV-CDAT/uvcdat). To |
| 65 | +contribute you will need to sign up for a [free GitHub |
| 66 | +account](https://github.com/signup/free). We use |
| 67 | +[Git](http://git-scm.com/) for version control to allow many people to |
| 68 | +work together on the project. |
| 69 | + |
| 70 | +Some great resources for learning git: |
| 71 | + |
| 72 | +- the [GitHub help pages](http://help.github.com/). |
| 73 | +- the [NumPy's |
| 74 | + documentation](http://docs.scipy.org/doc/numpy/dev/index.html). |
| 75 | +- Matthew Brett's |
| 76 | + [Pydagogue](http://matthew-brett.github.com/pydagogue/). |
| 77 | + |
| 78 | +### Getting Started with Git |
| 79 | + |
| 80 | +[GitHub has instructions](http://help.github.com/set-up-git-redirect) |
| 81 | +for installing git, setting up your SSH key, and configuring git. All |
| 82 | +these steps need to be completed before working seamlessly with your |
| 83 | +local repository and GitHub. |
| 84 | + |
| 85 | +### Forking |
| 86 | + |
| 87 | +You may want to fork UV-CDAT to work on the code if you have access to the repository, |
| 88 | +then just create a branch there instead. If you don't then follow these guidelines |
| 89 | +for forking UV-CDAT. Go to the [UV-CDAT project page](https://github.com/UV-CDAT/uvcdat) |
| 90 | +and hit the *fork* button. You will want to clone your fork to your machine: (HTTPS |
| 91 | +or SSH is preferred to git:// for security reasons). |
| 92 | + |
| 93 | + git clone git://github.com/UV-CDAT/uvcdat.git UV-CDAT-yourname |
| 94 | + cd UV-CDAT-yourname |
| 95 | + git remote add myuvcdat git@github.com:your-user-name/uvcdat.git |
| 96 | + |
| 97 | +This creates the directory UV-CDAT-yourname and connects your repository |
| 98 | +to the upstream (main project) *UV-CDAT* repository. |
| 99 | + |
| 100 | +You will also need to hook up Travis-CI to your GitHub (if you have forked) |
| 101 | +repository so the suite is automatically run when a Pull Request is submitted. |
| 102 | +Instructions are [here](http://about.travis-ci.org/docs/user/getting-started/). |
| 103 | + |
| 104 | +### Creating a Branch |
| 105 | + |
| 106 | +You want your master branch to reflect only production-ready code, so |
| 107 | +create a feature branch for making your changes. For example: |
| 108 | + |
| 109 | + git branch shiny-new-feature |
| 110 | + git checkout shiny-new-feature |
| 111 | + |
| 112 | +The above can be simplified to: |
| 113 | + |
| 114 | + git checkout -b shiny-new-feature |
| 115 | + |
| 116 | +This changes your working directory to the shiny-new-feature branch. |
| 117 | +Keep any changes in this branch specific to one bug or feature so it is |
| 118 | +clear what the branch brings to *UV-CDAT*. You can have many |
| 119 | +shiny-new-features and switch in between them using the git checkout |
| 120 | +command. |
| 121 | + |
| 122 | +### Making changes |
| 123 | +Before making your code changes, it is often necessary to build the code |
| 124 | +that was just checked out. The best way to develop *UV-CDAT* is to build |
| 125 | +using default settings: |
| 126 | + |
| 127 | + mkdir uvcdat-build |
| 128 | + cmake uvcdat-path-to-source |
| 129 | + make -jN |
| 130 | + |
| 131 | +Contributing to the documentation |
| 132 | +--------------------------------- |
| 133 | + |
| 134 | +If you're not the developer type, contributing to the documentation is |
| 135 | +still of huge value. You don't even have to be an expert on *UV-CDAT* to |
| 136 | +do so! Something as simple as pointing missing information or broken links |
| 137 | +will be of great value. |
| 138 | + |
| 139 | +Contributing to the code base |
| 140 | +----------------------------- |
| 141 | +### Code Standards |
| 142 | + |
| 143 | +*UV-CDAT* uses [flake8](http://pypi.python.org/pypi/flake8) tool for |
| 144 | +checking the style of your code. |
| 145 | + |
| 146 | +Please try to maintain backward-compatibility. *UV-CDAT* has lots of |
| 147 | +users with lots of existing code, so don't break it if at all possible. |
| 148 | +If you think breakage is required clearly state why as part of the Pull |
| 149 | +Request. Also, be careful when changing method signatures and add |
| 150 | +deprecation warnings where needed. |
| 151 | + |
| 152 | +### Test-driven Development/Writing Code |
| 153 | +*UV-CDAT* is serious about [Test-driven Development |
| 154 | +(TDD)](http://en.wikipedia.org/wiki/Test-driven_development). This |
| 155 | +development process "relies on the repetition of a very short |
| 156 | +development cycle: first the developer writes an (initially failing) |
| 157 | +automated test case that defines a desired improvement or new function, |
| 158 | +then produces the minimum amount of code to pass that test." So, before |
| 159 | +actually writing any code, you should write your tests. Often the test |
| 160 | +can be taken from the original GitHub issue. However, it is always worth |
| 161 | +considering additional use cases and writing corresponding tests. |
| 162 | + |
| 163 | +Adding tests is one of the most common requests after code is pushed to |
| 164 | +*UV-CDAT*. It is worth getting in the habit of writing tests ahead of |
| 165 | +time so this is never an issue. |
| 166 | + |
| 167 | +#### Writing tests |
| 168 | + |
| 169 | +All tests should go into the *tests* subdirectory of the specific |
| 170 | +package. There are probably many examples already there and looking to |
| 171 | +these for inspiration is suggested. |
| 172 | + |
| 173 | +#### Regression testing |
| 174 | + |
| 175 | +The `testing.checkimage.py` module has special `check_result_image` function |
| 176 | +that make it easier to check whether plot produced after data extraction and |
| 177 | +transformation are equivalent to baseline. For an example see below: |
| 178 | + |
| 179 | + import cdms2,sys,vcs,sys,os |
| 180 | + src = sys.argv[1] |
| 181 | + pth = os.path.join(os.path.dirname(__file__),"..") |
| 182 | + sys.path.append(pth) |
| 183 | + import checkimage |
| 184 | + x = vcs.init() |
| 185 | + x.drawlogooff() // It is important to disable logo for testing |
| 186 | + f = cdms2.open(vcs.prefix+"/sample_data/clt.nc") |
| 187 | + s = f("clt",slice(0,1),squeeze=1) |
| 188 | + b = x.createboxfill() |
| 189 | + b.level_1 = .5 |
| 190 | + b.level_2 = 14.5 |
| 191 | + x.plot(s,b,bg = 1) |
| 192 | + |
| 193 | + fnm = "test_boxfill_lev1_lev2.png" |
| 194 | + |
| 195 | + x.png(fnm) |
| 196 | + |
| 197 | + ret = checkimage.check_result_image(fnm,src,checkimage.defaultThreshold) |
| 198 | + sys.exit(ret) |
| 199 | + |
| 200 | +#### Running the test suite |
| 201 | + |
| 202 | +The tests can then be run directly inside your build tree (directory) by typing:: |
| 203 | + |
| 204 | + ctest |
| 205 | + |
| 206 | +To save time and run tests in Pararallel |
| 207 | + |
| 208 | + ctest -jN |
| 209 | + |
| 210 | +The tests suite is exhaustive and takes around 20 minutes to run. Often |
| 211 | +it is worth running only a subset of tests first around your changes |
| 212 | +before running the entire suite. This is done using one of the following |
| 213 | +constructs: |
| 214 | + |
| 215 | + ctest -R test-name |
| 216 | + ctest -R regex* |
| 217 | + |
| 218 | +Contributing your changes to *UV-CDAT* |
| 219 | +------------------------------------- |
| 220 | + |
| 221 | +### Committing your code |
| 222 | + |
| 223 | +Keep style fixes to a separate commit to make your PR more readable. Once you've made changes, you can see them by typing: |
| 224 | + |
| 225 | + git status |
| 226 | + |
| 227 | +If you've created a new file, it is not being tracked by git. Add it by |
| 228 | +typing : |
| 229 | + |
| 230 | + git add path/to/file-to-be-added.py |
| 231 | + |
| 232 | +Doing 'git status' again should give something like : |
| 233 | + |
| 234 | + # On branch shiny-new-feature |
| 235 | + # |
| 236 | + # modified: /relative/path/to/file-you-added.py |
| 237 | + # |
| 238 | + |
| 239 | +Finally, commit your changes to your local repository with an |
| 240 | +explanatory message. An informal commit message format is in effect for |
| 241 | +the project. Please try to adhere to it. Here are some common prefixes |
| 242 | +along with general guidelines for when to use them: |
| 243 | + |
| 244 | +> - ENH: Enhancement, new functionality |
| 245 | +> - BUG: Bug fix |
| 246 | +> - DOC: Additions/updates to documentation |
| 247 | +> - TST: Additions/updates to tests |
| 248 | +> - BLD: Updates to the build process/scripts |
| 249 | +> - PERF: Performance improvement |
| 250 | +> - CLN: Code cleanup |
| 251 | +
|
| 252 | +The following defines how a commit message should be structured. Please |
| 253 | +reference the relevant GitHub issues in your commit message using GH1234 |
| 254 | +or \#1234. Either style is fine, but the former is generally preferred: |
| 255 | + |
| 256 | +> - a subject line with \< 80 chars (50-char subject, 72-char rest). |
| 257 | +> - One blank line. |
| 258 | +> - Optionally, a commit message body (72-char). |
| 259 | +
|
| 260 | +Now you can commit your changes in your local repository: |
| 261 | + |
| 262 | + git commit -a |
| 263 | +or |
| 264 | + git commit -a -m "Message..Here" |
| 265 | + |
| 266 | +If you have multiple commits, it is common to want to combine them into |
| 267 | +one commit, often referred to as "squashing" or "rebasing". This is a |
| 268 | +common request by package maintainers when submitting a Pull Request as |
| 269 | +it maintains a more compact commit history. To rebase your commits: |
| 270 | + |
| 271 | + git rebase -i HEAD~# |
| 272 | + |
| 273 | +Where \# is the number of commits you want to combine. Then you can pick |
| 274 | +the relevant commit message and discard others. |
| 275 | + |
| 276 | +### Pushing your changes |
| 277 | + |
| 278 | +When you want your changes to appear publicly on your GitHub page, push |
| 279 | +your forked feature branch's commits : |
| 280 | + |
| 281 | + git push origin shiny-new-feature |
| 282 | + |
| 283 | +Here origin is the default name given to your remote repository on |
| 284 | +GitHub. You can see the remote repositories : |
| 285 | + |
| 286 | + git remote -v |
| 287 | + |
| 288 | +If you added the upstream repository as described above you will see |
| 289 | +something like : |
| 290 | + |
| 291 | + origin git://github.com/UV-CDAT/uvcdat.git |
| 292 | + myuvcdat git@github.com:yourname/uvcdat.git (fetch) |
| 293 | + myuvcdat git@github.com:yourname/uvcdat.git (fetch) |
| 294 | + |
| 295 | +Now your code is on GitHub, but it is not yet a part of the *UV-CDAT* |
| 296 | +project. For that to happen, a Pull Request needs to be submitted on |
| 297 | +GitHub. |
| 298 | + |
| 299 | +### Review your code |
| 300 | + |
| 301 | +When you're ready to ask for a code review, you will file a Pull |
| 302 | +Request. Before you do, again make sure you've followed all the |
| 303 | +guidelines outlined in this document regarding code style, tests, |
| 304 | +performance tests, and documentation. You should also double check your |
| 305 | +branch changes against the branch it was based off of: |
| 306 | + |
| 307 | +1. Navigate to your repository on |
| 308 | + GitHub--<https://github.com/your-user-name/uvcdat>. |
| 309 | +2. Click on Branches. |
| 310 | +3. Click on the Compare button for your feature branch. |
| 311 | +4. Select the base and compare branches, if necessary. This will be |
| 312 | + master and shiny-new-feature, respectively. |
| 313 | + |
| 314 | +### Finally, make the Pull Request |
| 315 | + |
| 316 | +If everything looks good you are ready to make a Pull Request. A Pull |
| 317 | +Request is how code from a local repository becomes available to the |
| 318 | +GitHub community and can be looked at and eventually merged into the |
| 319 | +master version. This Pull Request and its associated changes will |
| 320 | +eventually be committed to the master branch and available in the next |
| 321 | +release. To submit a Pull Request: |
| 322 | + |
| 323 | +1. Navigate to your repository on GitHub. |
| 324 | +2. Click on the Pull Request button. |
| 325 | +3. You can then click on Commits and Files Changed to make sure |
| 326 | + everything looks okay one last time. |
| 327 | +4. Write a description of your changes in the Preview Discussion tab. |
| 328 | +5. Click Send Pull Request. |
| 329 | + |
| 330 | +This request then appears to the repository maintainers, and they will |
| 331 | +review the code. If you need to make more changes, you can make them in |
| 332 | +your branch, push them to GitHub, and the pull request will be |
| 333 | +automatically updated. Pushing them to GitHub again is done by: |
| 334 | + |
| 335 | + git push -f myuvcdat shiny-new-feature |
| 336 | + |
| 337 | +This will automatically update your Pull Request with the latest code |
| 338 | +and restart the Travis-CI tests. |
| 339 | + |
| 340 | +### Delete your merged branch from your fork (optional) |
| 341 | + |
| 342 | +Once your feature branch is accepted into upstream, you'll probably want |
| 343 | +to get rid of the branch. First, merge upstream master into your branch |
| 344 | +so git knows it is safe to delete your branch : |
| 345 | + |
| 346 | + git fetch origin |
| 347 | + git checkout master |
| 348 | + git reset --hard origin/master |
| 349 | + |
| 350 | +Then you can just do: |
| 351 | + |
| 352 | + git branch -d shiny-new-feature |
| 353 | + |
| 354 | +Make sure you use a lower-case -d, or else git won't warn you if your |
| 355 | +feature branch has not actually been merged. |
| 356 | + |
| 357 | +The branch will still exist on GitHub, so to delete it there do : |
| 358 | + |
| 359 | + git push origin --delete shiny-new-feature |
0 commit comments