Merge pull request #680 from opdemand/add-contributing

Matthew Fisher · Matthew Fisher · commit ac647448207f · 2014-04-03T18:49:11.000-04:00
docs(standards): add commit style guide
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -0,0 +1,105 @@
+# How to Contribute
+
+The Deis project is Apache 2.0 licensed and accepts contributions via Github pull
+requests. This document outlines some of the conventions on commit message formatting,
+contact points for developers and other resources to make getting your contribution
+accepted.
+
+# Certificate of Origin
+
+By contributing to this project you agree to the Developer Certificate of Origin (DCO).
+This document was created by the Linux Kernel community and is a simple statement that
+you, as a contributor, have the legal right to make the contribution.
+
+```
+Developer Certificate of Origin
+Version 1.1
+
+Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
+660 York Street, Suite 102,
+San Francisco, CA 94110 USA
+
+Everyone is permitted to copy and distribute verbatim copies of this
+license document, but changing it is not allowed.
+
+
+Developer's Certificate of Origin 1.1
+
+By making a contribution to this project, I certify that:
+
+(a) The contribution was created in whole or in part by me and I
+    have the right to submit it under the open source license
+    indicated in the file; or
+
+(b) The contribution is based upon previous work that, to the best
+    of my knowledge, is covered under an appropriate open source
+    license and I have the right under that license to submit that
+    work with modifications, whether created in whole or in part
+    by me, under the same open source license (unless I am
+    permitted to submit under a different license), as indicated
+    in the file; or
+
+(c) The contribution was provided directly to me by some other
+    person who certified (a), (b) or (c) and I have not modified
+    it.
+
+(d) I understand and agree that this project and the contribution
+    are public and that a record of the contribution (including all
+    personal information I submit with it, including my sign-off) is
+    maintained indefinitely and may be redistributed consistent with
+    this project or the open source license(s) involved.
+```
+
+
+# Support Channels
+
+- IRC: #[deis](irc://irc.freenode.org:6667/#deis) IRC channel on freenode.org
+
+## Getting Started
+
+- Fork the repository on GitHub
+- Read the README.md for build instructions
+
+## Contribution Flow
+
+This is a rough outline of what a contributor's workflow looks like:
+
+- Create a topic branch from where you want to base your work. This is usually master.
+- Make commits of logical units.
+- Make sure your commit messages are in the proper format, see below
+- Push your changes to a topic branch in your fork of the repository.
+- Submit a pull request
+
+Thanks for your contributions!
+
+### Format of the Commit Message
+
+We follow a rough convention for commit messages borrowed from CoreOS, who borrowed theirs
+from AngularJS. This is an example of a commit:
+
+```
+    feat(scripts/test-cluster): add a cluster test command
+
+    this uses tmux to setup a test cluster that you can easily kill and
+    start for debugging.
+```
+
+To make it more formal it looks something like this:
+
+```
+{type}({scope}): {subject}
+<BLANK LINE>
+{body}
+<BLANK LINE>
+{footer}
+```
+
+Any line of the commit message cannot be longer than 90 characters, with the subject line
+limited to 70 characters. This allows the message to be easier to read on github as well
+as in various git tools.
+
+### More Details on Commits
+
+For more details see the [commit style guide][1].
+
+[1]: http://docs.deis.io/en/latest/contributing/standards/#commit-style-guide
diff --git a/docs/contributing/standards.rst b/docs/contributing/standards.rst
@@ -33,10 +33,8 @@ into the official repository, your code must pass two tests:
 
 .. code-block:: console
 
-	$ cd $HOME/projects/deis
-	$ make flake8
-	flake8
-	$
+    $ make -C controller flake8
+    flake8
 
 No output, as above, means ``flake8`` found no errors. If errors
 are reported, fix them in your source code and try ``flake8`` again.
@@ -60,18 +58,17 @@ ensure that everything passes and that code coverage has not declined.
 
 .. code-block:: console
 
-	$ cd $HOME/projects/deis
-	$ make coverage
-	coverage run manage.py test api celerytasks client web
-	Creating test database for alias 'default'...
-	...................ss.
-	----------------------------------------------------------------------
-	Ran 22 tests in 22.630s
+    $ make -C controller coverage
+    coverage run manage.py test --noinput api cm provider web
+    WARNING Cannot synchronize with etcd cluster
+    Creating test database for alias 'default'...
+    ....................................................................
+    ----------------------------------------------------------------------
+    Ran 68 tests in 84.856s
 
-	OK (skipped=2)
-	Destroying test database for alias 'default'...
-	coverage html
-	$
+    OK
+    Destroying test database for alias 'default'...
+    coverage html
 
 If a test fails, fixing it is obviously the first priority. And if you
 have introduced new code, it must be accompanied by unit tests.
@@ -91,6 +88,7 @@ changes. Current test coverage can be found here:
 
 Pull Request
 ------------
+
 Now create a GitHub `pull request`_ with a description of what your code
 fixes or improves.
 
@@ -105,6 +103,7 @@ automatically close the `GitHub issue`_ when the pull request is merged.
 
 Merge Approval
 --------------
+
 Deis maintainers add "**LGTM**" (Looks Good To Me) in code
 review comments to indicate that a PR is acceptable. Any code change--other than
 a simple typo fix or one-line documentation change--requires at least two of
@@ -119,3 +118,181 @@ Deis' maintainers to accept the change in this manner before it can be merged.
 .. _`The Zen of Python`: http://www.python.org/dev/peps/pep-0020/
 .. _`pull request`: https://github.com/opdemand/deis/pulls
 .. _`GitHub issue`: https://github.com/opdemand/deis/issues
+
+
+.. _commit_style_guide:
+
+Commit Style Guide
+------------------
+
+There are several reasons why we try to follow a specific style guide for commits:
+
+- it allows us to recognize unimportant commits like formatting
+- it provides better information when browsing the git history
+
+Recognizing Unimportant Commits
+```````````````````````````````
+
+These commits are usually just formatting changes like adding/removing spaces/empty lines,
+fixing indentation, or adding comments. So when you are looking for some change in the
+logic, you can ignore these commits - there's no logic change inside this commit.
+
+When bisecting, you can ignore these by running:
+
+.. code-block:: console
+
+    git bisect skip $(git rev-list --grep irrelevant <good place> HEAD)
+
+Providing more Information when Browsing the History
+````````````````````````````````````````````````````
+
+This adds extra context to our commit logs. Look at these messages (taken from the last
+few AngularJS commits):
+
+- Fix small typo in docs widget (tutorial instructions)
+- Fix test for scenario.Application - should remove old iframe
+- docs - various doc fixes
+- docs - stripping extra new lines
+- Replaced double line break with single when text is fetched from Google
+- Added support for properties in documentation
+
+All of these messages try to specify where the change occurs, but they don’t share any
+convention. Now look at these messages:
+
+- fix comment stripping
+- fixing broken links
+- Bit of refactoring
+- Check whether links do exist and throw exception
+- Fix sitemap include (to work on case sensitive linux)
+
+Are you able to guess what’s inside each commit diff?
+
+It's true that you can find this information by checking which files had been changed, but
+that’s slow. When looking in the git history, we can see that all of the developers are
+trying to specify where the change takes place, but the message is missing a convention.
+Cue commit message formatting entrance stage left.
+
+Format of the Commit Message
+````````````````````````````
+
+.. code-block:: console
+
+    {type}({scope}): {subject}
+    <BLANK LINE>
+    {body}
+    <BLANK LINE>
+    {footer}
+
+Any line of the commit message cannot be longer than 90 characters, with the subject
+line limited to 70 characters. This allows the message to be easier to read on github
+as well as in various git tools.
+
+Subject Line
+""""""""""""
+
+The subject line contains a succinct description of the change to the logic.
+
+The allowed {types} are as follows:
+
+- feat -> feature
+- fix -> bug fix
+- docs -> documentation
+- style -> formatting
+- refactor
+- test -> adding missing tests
+- chore -> maintenance
+
+The {scope} can be anything specifying place of the commit change e.g. the controller,
+the client, the logger, etc.
+
+The {subject} needs to use imperative, present tense: “change”, not “changed” nor
+“changes”. The first letter should not be capitalized, and there is no dot (.) at the end.
+
+Message Body
+""""""""""""
+
+Just like the {subject}, the message {body} needs to be in the present tense, and includes
+the motivation for the change, as well as a contrast with the previous behavior.
+
+Message Footer
+""""""""""""""
+
+All breaking changes need to be mentioned in the footer with the description of the
+change, the justification behind the change and any migration notes required. For example:
+
+.. code-block:: console
+
+    BREAKING CHANGE: the controller no longer listens on port 80. It now listens on
+        port 8000, with the router redirecting requests on port 80 to the controller. To
+        migrate to this change, SSH into your controller and run:
+
+        $ docker kill deis-controller
+        $ docker rm deis-controller
+
+        and then restart the controller on port 8000:
+
+        $ docker run -d -p 8000:8000 -e ETCD=<etcd_endpoint> -e HOST=<host_ip> \
+        -e PORT=8000 -name deis-controller deis/controller
+
+        now you can start the proxy component by running:
+
+        $ docker run -d -p 80:80 -e ETCD=<etcd_endpoint> -e HOST=<host_ip> -e PORT=80 \
+        -name deis-router deis/router
+
+        The router should then start proxying requests from port 80 to the controller.
+
+Referencing Issues
+""""""""""""""""""
+
+Closed bugs should be listed on a separate line in the footer prefixed with the "closes"
+keyword like this:
+
+.. code-block::console
+
+    closes #123
+
+Or in the case of multiple issues:
+
+.. code-block::console
+
+    closes #123, #456, #789
+
+Examples
+````````
+
+.. code-block:: console
+
+    feat(controller): add router component
+
+    This introduces a new router component to Deis, which proxies requests to Deis
+    components.
+
+    closes #123
+
+    BREAKING CHANGE: the controller no longer listens on port 80. It now listens on
+        port 8000, with the router redirecting requests on port 80 to the controller. To
+        migrate to this change, SSH into your controller and run:
+
+        $ docker kill deis-controller
+        $ docker rm deis-controller
+
+        and then restart the controller on port 8000:
+
+        $ docker run -d -p 8000:8000 -e ETCD=<etcd_endpoint> -e HOST=<host_ip> \
+        -e PORT=8000 -name deis-controller deis/controller
+
+        now you can start the proxy component by running:
+
+        $ docker run -d -p 80:80 -e ETCD=<etcd_endpoint> -e HOST=<host_ip> -e PORT=80 \
+        -name deis-router deis/router
+
+        The router should then start proxying requests from port 80 to the controller.
+    ----------------------------------------------------------------------------------
+    test(client): add unit tests for app domains
+
+    Nginx does not allow domain names larger than 128 characters, so we need to make
+    sure that we do not allow the client to add domains larger than 128 characters.
+    A DomainException is raised when the domain name is larger than the maximum
+    character size.
+
+    closes #392
