Skip to content

Commit

Permalink
Collect How to Contribute notes into one place
Browse files Browse the repository at this point in the history
Collect notes about how to contribute to CKAN (github fork, feature
branches, commit messages, pull requests & code review, merging) from
different parts of the Coding Standards docs page and the Submitting a
code patch and Becoming a CKAN Developer pages on the old wiki.ckan.org.
Put them all together at the top of the coding standards.

Removed the section about submitting code patches by email.
  • Loading branch information
Sean Hammond authored and tobes committed Nov 20, 2012
1 parent b61100b commit c79a4e5
Showing 1 changed file with 73 additions and 66 deletions.
139 changes: 73 additions & 66 deletions doc/coding-standards.rst
Expand Up @@ -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 <https://github.com/>`_ and
`fork CKAN <https://help.github.com/articles/fork-a-repo>`_, 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
=======================
Expand Down Expand Up @@ -591,6 +596,8 @@ When developing for ckan core, only use the helper functions found in

.. todo:: Jinja2 templates

.. _Testing:

Testing
-------

Expand Down

0 comments on commit c79a4e5

Please sign in to comment.