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