diff --git a/doc/coding-standards.rst b/doc/coding-standards.rst index d85dbdf169b..55bd4656491 100644 --- a/doc/coding-standards.rst +++ b/doc/coding-standards.rst @@ -2,111 +2,116 @@ CKAN Coding Standards ===================== -Commit Guidelines -================= -Generally, follow the `commit guidelines from the Pro Git book`_: +How to Contribute Code to CKAN +------------------------------ -- Try to make each commit a logically separate, digestible changeset. +CKAN is a free software project and code contributions are welcome. To +contribute code to CKAN you should fork CKAN to your own GitHub account, push +your code to a feature branch on your fork, then make a pull request for your +branch on the central CKAN repo. We'll go through each step in detail below... -- The first line of the commit message should concisely summarise the - changeset. -- Optionally, follow with a blank line and then a more detailed explanation of - the changeset. +Fork CKAN on GitHub +``````````````````` -- Use the imperative present tense as if you were giving commands to the - codebase to change its behaviour, e.g. *Add tests for...*, *make xyzzy do - frotz...*, this helps to make the commit message easy to read. +.. _CKAN repo on GitHub: https://github.com/okfn/ckan +.. _CKAN issue tracker: http://trac.ckan.org -- Try to write the commit message so that a new CKAN developer could understand - it, i.e. using plain English as far as possible, and not referring to too - much assumed knowledge or to external resources such as mailing list - discussions (summarize the relevant points in the commit message instead). +If you don't have access to the central `CKAN repo on GitHub`_ you should sign +up for a free account on `GitHub.com `_ and +`fork CKAN `_, so that you have somewhere to publish your CKAN code. -.. _commit guidelines from the Pro Git book: http://git-scm.com/book/en/Distributed-Git-Contributing-to-a-Project#Commit-Guidelines +You can now clone your CKAN fork to your development machine, create a new +branch to commit your code on, and push your branch to your CKAN fork on GitHub +to share your code with others. -In CKAN we also refer to `trac.ckan.org`_ ticket numbers in commit messages -wherever relevant. This makes the release manager's job much easier! Of -course, you don't have to reference a ticket from your commit message if there -isn't a ticket for it, e.g. if you find a typo in a docstring and quickly fix -it you wouldn't bother to create a ticket for this. -Put the ticket number in square brackets (e.g. ``[#123]``) at the start of the -first line of the commit message. You can also reference other Trac tickets -elsewhere in your commit message by just using the ticket number on its own -(e.g. ``see #456``). For example: +Feature Branches +```````````````` -:: +Work for a feature or bug fix should be developed on a feature or bug branch +forked from master. Each individual feature or bug fix should be developed on +its own branch. The name of the branch should include the ticket number (if +this work has a ticket in the `CKAN issue tracker`_), the branch type +("feature" or "bug"), and a brief one-line synopsis of the purpose of the +ticket, for example:: - [#2505] Update source install instructions - - Following feedback from markw (see #2406). + 2298-feature-add-sort-by-controls-to-search-page + 1518-bug-upload-file-with-spaces -.. _trac.ckan.org: http://trac.ckan.org/ +Naming branches this way makes it easy to search for a branch by its ticket +number using GitHub's web interface. -Longer example CKAN commit message: -:: +Commit Messages +``````````````` - [#2304] Refactor user controller a little - - Move initialisation of a few more template variables into - _setup_template_variables(), and change read(), edit(), and followers() to use - it. This removes some code duplication and fixes issues with the followers - count and follow button not being initialisd on all user controller pages. +Generally, follow the `commit guidelines from the Pro Git book`_: - Change new() to _not_ use _setup_template_variables() as it only needs - c.is_sysadmin and not the rest. +- Try to make each commit a logically separate, digestible changeset. - Also fix templates/user/layout.html so that the Followers tab appears on both - your own user page (when logged in) and on other user's pages. +- The first line of the commit message should concisely summarise the + changeset. +- Optionally, follow with a blank line and then a more detailed explanation of + the changeset. -Branches, Pull Requests and Code Reviews ----------------------------------------- +- Use the imperative present tense as if you were giving commands to the + codebase to change its behaviour, e.g. *Add tests for...*, *make xyzzy do + frotz...*, this helps to make the commit message easy to read. -Work for a ticket should be committed on a feature or bug branch forked -from master. The name of the branch should include the ticket's number, the -ticket type, and a brief one-line synopsis of the purpose of the ticket, e.g.:: +.. _commit guidelines from the Pro Git book: http://git-scm.com/book/en/Distributed-Git-Contributing-to-a-Project#Commit-Guidelines - 2298-feature-add-sort-by-controls-to-search-page - 1518-defect-upload-file-with-spaces +If your commit has a ticket in the `CKAN issue tracker`_ put the ticket number +at the start of the first line of the commit message like this: ``[#123]``. +This makes the CKAN release manager's job much easier! -Naming branches this way makes it easy to search for a branch by its ticket -number using GitHub's web interface. +Here is an example CKAN commit message:: -Once work on the branch has been completed and it is ready to be merged into -master, make a pull request on GitHub. Another member of the CKAN team will -review the changes, and provide feedback through the GitHub pull request page. + [#2505] Update source install instructions + Following feedback from markw (see #2406). -Submitting Code Patches ------------------------ -See the wiki for instructions on `how to submit a patch`_ via GitHub or email. +Pull Requests & Code Review +``````````````````````````` -.. _how to submit a patch: http://wiki.ckan.org/Submitting_a_code_patch +.. _create a pull request on GitHub: https://help.github.com/articles/creating-a-pull-request -Releases --------- +Once your work on a branch is complete and is ready to be merged into the +master branch, `create a pull request on GitHub`_. A member of the CKAN team +will review your code and provide feedback on the pull request page. The +reviewer may ask you to make some changes to your code. Once the pull request +has passed the code review, the reviewer will merge your code into the master +branch and it will become part of CKAN! + +.. note:: + + When submitting a pull request: + + - Your branch should contain code for one feature or bug fix only, + see `Feature Branches`_. + - Your branch should contain new or changed tests for any new or changed + code, see `Testing`_. + - All the CKAN tests should pass on your branch, see :doc:`test`. -See :doc:`release-cycle` for details on the release process. Merging -------- +``````` When merging a feature or bug branch into master: +- Make sure the tests pass, see :doc:`test` - Use the ``--no-ff`` option in the ``git merge`` command - Add an entry to the ``CHANGELOG`` file -The full postgresql test suite must pass before merging into master. :: - nosetests --ckan --with-pylons=test-core.ini ckan +Releases +-------- + +See :doc:`release-cycle` for details on the release process. -See :doc:`test` for more information on running tests, including running the -core extension tests. Python Coding Standards ======================= @@ -591,6 +596,8 @@ When developing for ckan core, only use the helper functions found in .. todo:: Jinja2 templates +.. _Testing: + Testing -------