Show More
@@ -1,177 +1,177 b'' | |||
|
1 | 1 | |
|
2 | 2 | ====================== |
|
3 | 3 | Contribution Standards |
|
4 | 4 | ====================== |
|
5 | 5 | |
|
6 | 6 | Standards help to improve the quality of our product and its development. Herein |
|
7 | 7 | we define our standards for processes and development to maintain consistency |
|
8 | 8 | and function well as a community. It is a work in progress; modifications to this |
|
9 | 9 | document should be discussed and agreed upon by the community. |
|
10 | 10 | |
|
11 | 11 | |
|
12 | 12 | -------------------------------------------------------------------------------- |
|
13 | 13 | |
|
14 | 14 | Code |
|
15 | 15 | ==== |
|
16 | 16 | |
|
17 | 17 | This provides an outline for standards we use in our codebase to keep our code |
|
18 | 18 | easy to read and easy to maintain. Much of our code guidelines are based on the |
|
19 | 19 | book `Clean Code <http://www.pearsonhighered.com/educator/product/Clean-Code-A-Handbook-of-Agile-Software-Craftsmanship/9780132350884.page>`_ |
|
20 | 20 | by Robert Martin. |
|
21 | 21 | |
|
22 | 22 | We maintain a Tech Glossary to provide consistency in terms and symbolic names |
|
23 | 23 | used for items and concepts within the application. This is found in the CE |
|
24 | 24 | project in /docs-internal/glossary.rst |
|
25 | 25 | |
|
26 | 26 | |
|
27 | 27 | Refactoring |
|
28 | 28 | ----------- |
|
29 | 29 | Make it better than you found it! |
|
30 | 30 | |
|
31 | 31 | Our codebase can always use improvement and often benefits from refactoring. |
|
32 | 32 | New code should be refactored as it is being written, and old code should be |
|
33 | 33 | treated with the same care as if it was new. Before doing any refactoring, |
|
34 | 34 | ensure that there is test coverage on the affected code; this will help |
|
35 | 35 | minimize issues. |
|
36 | 36 | |
|
37 | 37 | |
|
38 | 38 | Python |
|
39 | 39 | ------ |
|
40 | 40 | For Python, we use `PEP8 <https://www.python.org/dev/peps/pep-0008/>`_. |
|
41 | 41 | We adjust lines of code to under 80 characters and use 4 spaces for indentation. |
|
42 | 42 | |
|
43 | 43 | |
|
44 | 44 | JavaScript |
|
45 | 45 | ---------- |
|
46 | 46 | This currently remains undefined. Suggestions welcome! |
|
47 | 47 | |
|
48 | 48 | |
|
49 | 49 | HTML |
|
50 | 50 | ---- |
|
51 | 51 | Unfortunately, we currently have no strict HTML standards, but there are a few |
|
52 | 52 | guidelines we do follow. Templates must work in all modern browsers. HTML should |
|
53 | 53 | be clean and easy to read, and additionally should be free of inline CSS or |
|
54 | 54 | JavaScript. It is recommended to use data attributes for JS actions where |
|
55 | 55 | possible in order to separate it from styling and prevent unintentional changes. |
|
56 | 56 | |
|
57 | 57 | |
|
58 | 58 | LESS/CSS |
|
59 | 59 | -------- |
|
60 | 60 | We use LESS for our CSS; see :doc:`frontend` for structure and formatting |
|
61 | 61 | guidelines. |
|
62 | 62 | |
|
63 | 63 | |
|
64 | 64 | Linters |
|
65 | 65 | ------- |
|
66 | 66 | Currently, we have a linter for pull requests which checks code against PEP8. |
|
67 | 67 | We intend to add more in the future as we clarify standards. |
|
68 | 68 | |
|
69 | 69 | |
|
70 | 70 | -------------------------------------------------------------------------------- |
|
71 | 71 | |
|
72 | 72 | Naming Conventions |
|
73 | 73 | ================== |
|
74 | 74 | |
|
75 | 75 | These still need to be defined for naming everything from Python variables to |
|
76 | 76 | HTML classes to files and folders. |
|
77 | 77 | |
|
78 | 78 | |
|
79 | 79 | -------------------------------------------------------------------------------- |
|
80 | 80 | |
|
81 | 81 | Testing |
|
82 | 82 | ======= |
|
83 | 83 | |
|
84 | 84 | Testing is a very important aspect of our process, especially as we are our own |
|
85 | 85 | quality control team. While it is of course unrealistic to hit every potential |
|
86 | 86 | combination, our goal is to cover every line of Python code with a test. |
|
87 | 87 | |
|
88 | 88 | The following is a brief introduction to our test suite. Our tests are primarily |
|
89 | 89 | written using `py.test <http://pytest.org/>`_ |
|
90 | 90 | |
|
91 | 91 | |
|
92 | 92 | Acceptance Tests |
|
93 | 93 | ---------------- |
|
94 | 94 | Also known as "ac tests", these test from the user and business perspective to |
|
95 | 95 | check if the requirements of a feature are met. Scenarios are created at a |
|
96 | 96 | feature's inception and help to define its value. |
|
97 | 97 | |
|
98 |
py.test is used for ac tests; they are located in a |
|
|
98 | py.test is used for ac tests; they are located in a repo separate from the | |
|
99 | 99 | other tests which follow. Each feature has a .feature file which contains a |
|
100 | 100 | brief description and the scenarios to be tested. |
|
101 | 101 | |
|
102 | 102 | |
|
103 | 103 | Functional Tests |
|
104 | 104 | ---------------- |
|
105 | 105 | These test specific functionality in the application which checks through the |
|
106 | 106 | entire stack. Typically these are user actions or permissions which go through |
|
107 | 107 | the web browser. They are located in rhodecode/tests. |
|
108 | 108 | |
|
109 | 109 | |
|
110 | 110 | Unit Tests |
|
111 | 111 | ---------- |
|
112 | 112 | These test isolated, individual modules to ensure that they function correctly. |
|
113 | 113 | They are located in rhodecode/tests. |
|
114 | 114 | |
|
115 | 115 | |
|
116 | 116 | Integration Tests |
|
117 | 117 | ----------------- |
|
118 | 118 | These are used for testing performance of larger systems than the unit tests. |
|
119 | 119 | They are located in rhodecode/tests. |
|
120 | 120 | |
|
121 | 121 | |
|
122 | 122 | JavaScript Testing |
|
123 | 123 | ------------------ |
|
124 | 124 | Currently, we have not defined how to test our JavaScript code. |
|
125 | 125 | |
|
126 | 126 | |
|
127 | 127 | -------------------------------------------------------------------------------- |
|
128 | 128 | |
|
129 | 129 | Pull Requests |
|
130 | 130 | ============= |
|
131 | 131 | |
|
132 | 132 | Pull requests should generally contain only one thing: a single feature, one bug |
|
133 | 133 | fix, etc.. The commit history should be concise and clean, and the pull request |
|
134 | 134 | should contain the ticket number (also a good idea for the commits themselves) |
|
135 | 135 | to provide context for the reviewer. |
|
136 | 136 | |
|
137 | 137 | See also: :doc:`checklist-pull-request` |
|
138 | 138 | |
|
139 | 139 | |
|
140 | 140 | Reviewers |
|
141 | 141 | --------- |
|
142 | 142 | Each pull request must be approved by at least one member of the RhodeCode |
|
143 | 143 | team. Assignments may be based on expertise or familiarity with a particular |
|
144 | 144 | area of code, or simply availability. Switching up or adding extra community |
|
145 | 145 | members for different pull requests helps to share knowledge as well as provide |
|
146 | 146 | other perspectives. |
|
147 | 147 | |
|
148 | 148 | |
|
149 | 149 | Responsibility |
|
150 | 150 | -------------- |
|
151 | 151 | The community is responsible for maintaining features and this must be taken |
|
152 | 152 | into consideration. External contributions must be held to the same standards |
|
153 | 153 | as internal contributions. |
|
154 | 154 | |
|
155 | 155 | |
|
156 | 156 | Feature Switch |
|
157 | 157 | -------------- |
|
158 | 158 | Experimental and work-in-progress features can be hidden behind one of two |
|
159 | 159 | switches: |
|
160 | 160 | |
|
161 | 161 | * A setting can be added to the Labs page in the Admin section which may allow |
|
162 | 162 | customers to access and toggle additional features. |
|
163 | 163 | * For work-in-progress or other features where customer access is not desired, |
|
164 | 164 | use a custom setting in the .ini file as a trigger. |
|
165 | 165 | |
|
166 | 166 | |
|
167 | 167 | -------------------------------------------------------------------------------- |
|
168 | 168 | |
|
169 | 169 | Tickets |
|
170 | 170 | ======= |
|
171 | 171 | |
|
172 | 172 | Redmine tickets are a crucial part of our development process. Any code added or |
|
173 | 173 | changed in our codebase should have a corresponding ticket to document it. With |
|
174 | 174 | this in mind, it is important that tickets be as clear and concise as possible, |
|
175 | 175 | including what the expected outcome is. |
|
176 | 176 | |
|
177 | 177 | See also: :doc:`checklist-tickets` |
General Comments 0
You need to be logged in to leave comments.
Login now