@@ -33,10 +33,10 @@ into the official repository, your code must pass two tests:
3333
3434.. code-block :: console
3535
36- $ cd $HOME/projects/deis
37- $ make flake8
38- flake8
39- $
36+ $ cd $HOME/projects/deis
37+ $ make flake8
38+ flake8
39+ $
4040
4141 No output, as above, means ``flake8 `` found no errors. If errors
4242are reported, fix them in your source code and try ``flake8 `` again.
@@ -60,18 +60,18 @@ ensure that everything passes and that code coverage has not declined.
6060
6161.. code-block :: console
6262
63- $ cd $HOME/projects/deis
64- $ make coverage
65- coverage run manage.py test api celerytasks client web
66- Creating test database for alias 'default'...
67- ...................ss.
68- ----------------------------------------------------------------------
69- Ran 22 tests in 22.630s
63+ $ cd $HOME/projects/deis
64+ $ make coverage
65+ coverage run manage.py test api celerytasks client web
66+ Creating test database for alias 'default'...
67+ ...................ss.
68+ ----------------------------------------------------------------------
69+ Ran 22 tests in 22.630s
7070
71- OK (skipped=2)
72- Destroying test database for alias 'default'...
73- coverage html
74- $
71+ OK (skipped=2)
72+ Destroying test database for alias 'default'...
73+ coverage html
74+ $
7575
7676 If a test fails, fixing it is obviously the first priority. And if you
7777have introduced new code, it must be accompanied by unit tests.
@@ -91,6 +91,7 @@ changes. Current test coverage can be found here:
9191
9292Pull Request
9393------------
94+
9495Now create a GitHub `pull request `_ with a description of what your code
9596fixes or improves.
9697
@@ -105,6 +106,7 @@ automatically close the `GitHub issue`_ when the pull request is merged.
105106
106107Merge Approval
107108--------------
109+
108110Deis maintainers add "**LGTM **" (Looks Good To Me) in code
109111review comments to indicate that a PR is acceptable. Any code change--other than
110112a simple typo fix or one-line documentation change--requires at least two of
@@ -119,3 +121,180 @@ Deis' maintainers to accept the change in this manner before it can be merged.
119121.. _`The Zen of Python` : http://www.python.org/dev/peps/pep-0020/
120122.. _`pull request` : https://github.com/opdemand/deis/pulls
121123.. _`GitHub issue` : https://github.com/opdemand/deis/issues
124+
125+
126+ .. _commit_style_guide :
127+
128+ Commit Style Guide
129+ ------------------
130+
131+ There are several reasons why we try to follow a specific style guide for commits:
132+
133+ - it allows us to recognize unimportant commits like formatting
134+ - it provides better information when browsing the git history
135+
136+ Recognizing Unimportant Commits
137+ ```````````````````````````````
138+
139+ These commits are usually just formatting changes like adding/removing spaces/empty lines,
140+ fixing indentation, or adding comments. So when you are looking for some change in the
141+ logic, you can ignore these commits - there's no logic change inside this commit.
142+
143+ When bisecting, you can ignore these by running:
144+
145+ .. code-block :: console
146+
147+ git bisect skip $(git rev-list --grep irrelevant <good place> HEAD)
148+
149+ Providing more Information when Browsing the History
150+ ````````````````````````````````````````````````````
151+
152+ This would add kinda “context” information. Look at these messages (taken from last few
153+ angular’s commits):
154+
155+ - Fix small typo in docs widget (tutorial instructions)
156+ - Fix test for scenario.Application - should remove old iframe
157+ - docs - various doc fixes
158+ - docs - stripping extra new lines
159+ - Replaced double line break with single when text is fetched from Google
160+ - Added support for properties in documentation
161+
162+ All of these messages try to specify where is the change. But they don’t share any
163+ convention. Look at these messages:
164+
165+ - fix comment stripping
166+ - fixing broken links
167+ - Bit of refactoring
168+ - Check whether links do exist and throw exception
169+ - Fix sitemap include (to work on case sensitive linux)
170+
171+ Are you able to guess what’s inside?
172+
173+ It's true that you can find this information by checking which files had been changed, but
174+ that’s slow. And when looking in git history I can see all of us tries to specify the
175+ place, only missing the convention. Cue commit message formatting entrance stage left.
176+
177+ Format of the Commit Message
178+ ````````````````````````````
179+
180+ .. code-block :: console
181+
182+ {type}({scope}): {subject}
183+ <BLANK LINE>
184+ {body}
185+ <BLANK LINE>
186+ {footer}
187+
188+ Any line of the commit message cannot be longer than 90 characters, with the subject
189+ line limited to 70 characters. This allows the message to be easier to read on github
190+ as well as in various git tools.
191+
192+ Subject Line
193+ """"""""""""
194+
195+ The subject line contains a succinct description of the change to the logic.
196+
197+ The allowed {types} are as follows:
198+
199+ - feat -> feature
200+ - fix -> bug fix
201+ - docs -> documentation
202+ - style -> formatting
203+ - refactor
204+ - test -> adding missing tests
205+ - chore -> maintenance
206+
207+ The {scope} can be anything specifying place of the commit change e.g. the controller,
208+ the client, the logger, etc.
209+
210+ The {subject} needs to use imperative, present tense: “change”, not “changed” nor
211+ “changes”. The first letter should not be capitalized, and there is no dot (.) at the end.
212+
213+ Message Body
214+ """"""""""""
215+
216+ Just like the {subject}, the message {body} needs to be in the present tense, and includes
217+ the motivation for the change, as well as a contrast with the previous behavior.
218+
219+ Message Footer
220+ """"""""""""""
221+
222+ All breaking changes need to be mentioned in the footer with the description of the
223+ change, the justification behind the change and any migration notes required. For example:
224+
225+ .. code-block :: console
226+
227+ BREAKING CHANGE: the controller no longer listens on port 80. It now listens on
228+ port 8000, with the router redirecting requests on port 80 to the controller. To
229+ migrate to this change, SSH into your controller and run:
230+
231+ $ docker kill deis-controller
232+ $ docker rm deis-controller
233+
234+ and then restart the controller on port 8000:
235+
236+ $ docker run -d -p 8000:8000 -e ETCD=<etcd_endpoint> -e HOST=<host_ip> \
237+ -e PORT=8000 -name deis-controller deis/controller
238+
239+ now you can start the proxy component by running:
240+
241+ $ docker run -d -p 80:80 -e ETCD=<etcd_endpoint> -e HOST=<host_ip> -e PORT=80 \
242+ -name deis-router deis/router
243+
244+ The router should then start proxying requests from port 80 to the controller.
245+
246+ Referencing Issues
247+ """"""""""""""""""
248+
249+ Closed bugs should be listed on a separate line in the footer prefixed with the "closes"
250+ keyword like this:
251+
252+ .. code-block::console
253+
254+ closes #123
255+
256+ Or in the case of multiple issues:
257+
258+ .. code-block::console
259+
260+ closes #123, #456, #789
261+
262+ Examples
263+ ````````
264+
265+ .. code-block :: console
266+
267+ feat(controller): add router component
268+
269+ This introduces a new router component to Deis, which proxies requests to Deis
270+ components.
271+
272+ closes #123
273+
274+ BREAKING CHANGE: the controller no longer listens on port 80. It now listens on
275+ port 8000, with the router redirecting requests on port 80 to the controller. To
276+ migrate to this change, SSH into your controller and run:
277+
278+ $ docker kill deis-controller
279+ $ docker rm deis-controller
280+
281+ and then restart the controller on port 8000:
282+
283+ $ docker run -d -p 8000:8000 -e ETCD=<etcd_endpoint> -e HOST=<host_ip> \
284+ -e PORT=8000 -name deis-controller deis/controller
285+
286+ now you can start the proxy component by running:
287+
288+ $ docker run -d -p 80:80 -e ETCD=<etcd_endpoint> -e HOST=<host_ip> -e PORT=80 \
289+ -name deis-router deis/router
290+
291+ The router should then start proxying requests from port 80 to the controller.
292+ ----------------------------------------------------------------------------------
293+ test(client): add unit tests for app domains
294+
295+ Nginx does not allow domain names larger than 128 characters, so we need to make
296+ sure that we do not allow the client to add domains larger than 128 characters.
297+ A DomainException is raised when the domain name is larger than the maximum
298+ character size.
299+
300+ closes #392
0 commit comments