]>
Commit | Line | Data |
---|---|---|
f6ee5b52 DL |
1 | # Developing for PROJECT (DRAFT) |
2 | ||
4765f35e | 3 | [TOC] |
f6ee5b52 | 4 | |
02fe6f86 DL |
5 | ## General note on this document |
6 | ||
7 | This document is "descriptive/post-factual" in that it documents pratices that | |
8 | are in use; it is not "definitive/pre-factual" in prescribing practices. | |
9 | ||
10 | This means that when a procedure changes, it is agreed upon, then put into | |
11 | practice, and then documented here. If this document doesn't match reality, | |
12 | it's the document that needs to be updated, not reality. | |
13 | ||
14 | ||
f6ee5b52 DL |
15 | ## Git Structure |
16 | ||
17 | The master Git for PROJECT resides on Github at | |
4765f35e | 18 | [https://github.com/PROJECT/XXX](https://github.com/PROJECT/XXX) |
f6ee5b52 | 19 | |
9b7939d9 DL |
20 | ![git branches continually merging to the left from 3 lanes; float-right](doc/git_branches.svg |
21 | "git branch mechanics") | |
22 | ||
02fe6f86 | 23 | There is one main branch for development and a release branch for each |
f6ee5b52 DL |
24 | major release. |
25 | ||
02fe6f86 | 26 | New contributions are done against the head of the master branch. The CI |
f6ee5b52 | 27 | systems will pick up the Github Pull Requests or the new patch from |
02fe6f86 | 28 | Patchwork, run some basic build and functional tests. |
f6ee5b52 DL |
29 | |
30 | For each major release (1.0, 1.1 etc) a new release branch is created based | |
31 | on the master. | |
32 | ||
02fe6f86 DL |
33 | There was an attempt to use a "develop" branch automatically maintained by |
34 | the CI system. This is not currently in active use, though the system is | |
35 | operational. If the "develop" branch is in active use and this paragraph | |
36 | is still here, this document obviously wasn't updated. | |
37 | ||
f6ee5b52 DL |
38 | |
39 | ## Programming language, Tools and Libraries | |
40 | ||
41 | The core of PROJECT is written in C (gcc or clang supported). A few | |
42 | non-essential scripts are implemented in Perl and Python. PROJECT requires | |
43 | the following tools to build distribution packages: automake, autoconf, | |
44 | texinfo, libtool and gawk and various libraries (i.e. libpam and libjson-c). | |
45 | ||
46 | If your contribution requires a new library or other tool, then please | |
47 | highlight this in your description of the change. Also make sure it’s | |
48 | supported by all PROJECT platform OSes or provide a way to build without the | |
49 | library (potentially without the new feature) on the other platforms. | |
50 | ||
51 | Documentation should be written in Tex (.texi) or Markdown (.md) format with | |
52 | preference on Markdown. | |
53 | ||
54 | ||
55 | ## Before Submitting your changes | |
56 | ||
4765f35e DL |
57 | * Format code (see [Code Styling requirements](#code-styling-requirements)) |
58 | * Verify and acknowledge license (see [License for contributions](#license-for-contributions)) | |
f6ee5b52 | 59 | * Test building with various configurations: |
4765f35e | 60 | * `buildtest.sh` |
f6ee5b52 | 61 | * Verify building source distribution: |
4765f35e | 62 | * `make dist` (and try rebuilding from the resulting tar file) |
f6ee5b52 | 63 | * Run DejaGNU unit tests: |
4765f35e | 64 | * `make test` |
f6ee5b52 DL |
65 | * Document Regression Runs and plans for continued maintenance of the feature |
66 | ||
67 | ### Changelog | |
68 | ||
69 | The changelog will be the base for the release notes. A changelog entry for | |
70 | your changes is usually not required and will be added based on your commit | |
71 | messages by the maintainers. However, you are free to include an update to | |
72 | the changelog with some better description. The changelog will be the base | |
73 | for the release notes. | |
74 | ||
75 | ||
76 | ## Submitting Patches and Enhancements | |
77 | ||
78 | ### License for contributions | |
79 | ||
80 | PROJECT is under a “GPLv2 or later” license. Any code submitted must be | |
81 | released under the same license (preferred) or any license which allows | |
82 | redistribution under this GPLv2 license (eg MIT License). | |
83 | ||
84 | ### Signed-off required | |
85 | ||
86 | Submissions to PROJECT require a “Signed-off” in the patch or git commit. | |
87 | We follow the same standard as the Linux Kernel Development. | |
88 | ||
89 | > Developer's Certificate of Origin 1.1 | |
90 | > | |
91 | > By making a contribution to this project, I certify that: | |
92 | > | |
93 | > (a) The contribution was created in whole or in part by me and I | |
94 | > have the right to submit it under the open source license | |
95 | > indicated in the file; or | |
96 | > | |
97 | > (b) The contribution is based upon previous work that, to the best | |
98 | > of my knowledge, is covered under an appropriate open source | |
99 | > license and I have the right under that license to submit that | |
100 | > work with modifications, whether created in whole or in part | |
101 | > by me, under the same open source license (unless I am | |
102 | > permitted to submit under a different license), as indicated | |
103 | > in the file; or | |
104 | > | |
105 | > (c) The contribution was provided directly to me by some other | |
106 | > person who certified (a), (b) or (c) and I have not modified | |
107 | > it. | |
108 | > | |
109 | > (d) I understand and agree that this project and the contribution | |
110 | > are public and that a record of the contribution (including all | |
111 | > personal information I submit with it, including my sign-off) is | |
112 | > maintained indefinitely and may be redistributed consistent with | |
113 | > this project or the open source license(s) involved. | |
114 | ||
115 | #### Using this Process | |
116 | ||
117 | We have the same requirements for using the signed-off-by process as the Linux | |
118 | kernel. In short, you need to include a signed-off-by tag in every patch: | |
119 | ||
120 | * `Signed-off-by:` this is a developer's certification that he or she has the | |
121 | right to submit the patch for inclusion into the project. It is an agreement to | |
122 | the Developer's Certificate of Origin (above). Code without a proper signoff | |
123 | cannot be merged into the mainline. | |
124 | ||
125 | Please make sure to have a `Signed-off-by:` in each commit/patch or the patches | |
126 | will be rejected until this is added. | |
127 | ||
128 | If you are unfamiliar with this process, you should read the [official policy | |
129 | at kernel.org](http://www.kernel.org/doc/Documentation/SubmittingPatches) and | |
130 | you might find this article about [participating in the Linux community on the | |
131 | Linux Foundation | |
132 | website](http://www.linuxfoundation.org/content/how-participate-linux-community-0) | |
133 | to be a helpful resource. | |
134 | ||
135 | ||
4765f35e | 136 | ### Code submission - Github Pull Request (Strongly Preferred) |
f6ee5b52 DL |
137 | |
138 | Preferred submission of code is by using a Github Pull Request against the | |
139 | Develop branch. Code submitted by Pull Request will have an email generated to | |
140 | the PROJECT-devel mailing list for review and the submission will be | |
141 | automatically tested by one or more CI systems. Only after this test succeeds | |
142 | (and the submission is based on the head of the develop branch), then it will | |
143 | be automatically merged into the develop branch. In case of failed tests, it is | |
144 | up to the submitter to either amend the request with further commits or close, | |
145 | fix and create a new pull request. | |
146 | ||
147 | Further (manual) code review and discussion happens after the merge into the | |
148 | develop branch. | |
149 | ||
150 | ||
4765f35e | 151 | ### Code submission - Mailing Patch to PROJECT-Devel list |
f6ee5b52 DL |
152 | |
153 | As an alternative submission, a patch can be mailed to the PROJECT-Devel | |
154 | mailing list. Preferred way to send the patch is using git send-mail. Patches | |
155 | received on the mailing list will be picked up by Patchwork and tested against | |
156 | the latest develop branch. After a further ACK by someone on the mailing list, | |
157 | the patch is then merged into the develop branch. | |
158 | ||
159 | Further (manual) code review and discussion happens after the merge into the | |
160 | develop branch. | |
161 | ||
4765f35e | 162 | #### Sending patch to mailing list |
f6ee5b52 DL |
163 | |
164 | The recommended way to send the patch (or series of NN patches) to the list is | |
165 | by using ‘git send-email’ as follows (assuming they are the most recent NN | |
166 | commit(s) in your git history: | |
167 | ||
168 | ``` | |
169 | git send-email -NN --annotate --to=XXX-Devel@XXX.org | |
170 | ``` | |
171 | ||
172 | If your commits do not already contain a `Signed-off-by` line, then use the | |
173 | following version to add it (after making sure to be able to agree to the | |
174 | Developer Certificate of Origin as outlined above): | |
175 | ||
176 | ``` | |
177 | git send-email -NN --annotate --signoff --to=XXX-Devel@XXX.org | |
178 | ``` | |
179 | ||
180 | Submitting multi-commit patches as a Github Pull Request is strongly encouraged | |
181 | and will allow your changes to merge faster | |
182 | ||
183 | ||
184 | ## After submitting your changes | |
185 | ||
186 | * Watch for Continuous Integration (CI) Test results | |
187 | * You should automatically receive an email with the test results within | |
188 | less than 2 hrs of the submission. If you don’t get the email, then check | |
189 | status on the github pull request (if submitted by pull request) or on | |
4765f35e DL |
190 | Patchwork at |
191 | [https://patchwork.PROJECT.org](https://patchwork.PROJECT.org) (if | |
192 | submitted as patch to mailing list). | |
f6ee5b52 | 193 | * Please notify PROJECT-Devel mailing list if you think something doesn’t |
4765f35e | 194 | work |
f6ee5b52 DL |
195 | * If the tests failed: |
196 | * In general, expect the community to ignore the submission until the tests | |
197 | pass. | |
198 | * It is up to you to fix and resubmit. | |
199 | * This includes fixing existing dejagnu (“make test”) tests if your | |
200 | changes broke or changed them. | |
201 | * It also includes fixing distribution packages for the failing | |
202 | platforms (ie if new libraries are required) | |
203 | * Feel free to ask for help on PROJECT-Devel list | |
204 | * Go back to the submission process and repeat until the tests pass. | |
205 | * If the tests pass: | |
206 | * If the changes are done as a pull request, then they should be | |
207 | automatically merged to the develop branch. | |
208 | * Changes sent to mailing list require a manual ACK to be merged and should | |
209 | be merged within 2 weeks. If you don’t see the merge or any | |
210 | reason/discussion on PROJECT-Devel, then please ask. | |
211 | * Watch out for questions on the mailing list. At this time there will be a | |
212 | manual code review and further (longer) tests by various community members. | |
213 | * Your submission is done once it is merged to the master branch. (which should | |
214 | happen every few weeks from the develop branch) | |
215 | ||
216 | ||
217 | ## Code Styling requirements | |
218 | ||
219 | ### File header required for new files added | |
220 | ||
4765f35e DL |
221 | New files need to have a Copyright header (see [License for |
222 | contributions](#license-for-contributions) above) added to the file. Preferred | |
223 | form of the header is as follows: | |
f6ee5b52 DL |
224 | |
225 | ``` | |
226 | /* | |
227 | Title/Function of file | |
228 | Copyright (C) 2016 Author’s Name | |
229 | ||
230 | This program is free software; you can redistribute it and/or modify | |
231 | it under the terms of the GNU General Public License as published by | |
232 | the Free Software Foundation; either version 2 of the License, or | |
233 | (at your option) any later version. | |
234 | ||
235 | This program is distributed in the hope that it will be useful, but | |
236 | WITHOUT ANY WARRANTY; without even the implied warranty of | |
237 | MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU | |
238 | General Public License for more details. | |
239 | ||
240 | You should have received a copy of the GNU General Public License | |
241 | along with this program; see the file COPYING; if not, write to the | |
242 | Free Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, | |
243 | MA 02110-1301 USA | |
244 | */ | |
4765f35e DL |
245 | |
246 | #include <zebra.h> | |
f6ee5b52 DL |
247 | ``` |
248 | ||
249 | ### Adding Copyright claims to already existing file | |
250 | ||
251 | When adding copyright claims for modifications to an existing file, please | |
252 | preface the claim with "Portions: " on a line before it and indent the | |
253 | "Copyright ..." string. If such a case already exists, add your indented claim | |
254 | immediately after. E.g.: | |
255 | ||
256 | ``` | |
257 | Portions: | |
258 | Copyright (C) 2010 Entity A .... | |
259 | Copyright (C) 2016 Your name [optional brief change description] | |
260 | ``` | |
261 | ||
262 | ### Code styling / format | |
263 | ||
02fe6f86 DL |
264 | Coding style standards in FRR vary depending on location. Pre-existing |
265 | code uses GNU coding standards. New code may use Linux kernel coding style. | |
266 | ||
267 | GNU coding style apply to the following parts: | |
268 | ||
269 | * lib/ | |
270 | * zebra/ | |
271 | * bgpd/ | |
272 | * ospfd/ | |
273 | * ospf6d/ | |
274 | * isisd/ | |
275 | * ripd/ | |
276 | * ripngd/ | |
277 | * vtysh/ | |
278 | ||
279 | Linux kernel coding style applies to: | |
280 | ||
281 | * nhrpd/ | |
282 | * watchfrr/ | |
283 | * pimd/ | |
284 | * lib/{checksum,hook,imsg-buffer,imsg,libfrr,md5,module,monotime,queue}.[ch] | |
285 | ||
286 | BSD coding style applies to: | |
287 | ||
288 | * ldpd/ | |
289 | ||
290 | **Whitespace changes in untouched parts of the code are not acceptable in | |
291 | patches that change actual code.** To change/fix formatting issues, please | |
292 | create a separate patch that only does formatting changes and nothing else. | |
293 | ||
294 | It is acceptable to rewrap entire files to Linux kernel style, but this | |
295 | **MUST** come as a separate patch that does nothing other than this | |
296 | reformatting. | |
297 | ||
298 | ||
299 | #### GNU style | |
300 | ||
301 | For GNU coding style, Indentation follows the result of invoking GNU indent: | |
f6ee5b52 DL |
302 | |
303 | ``` | |
304 | indent -nut -nfc1 file_for_submission.c | |
305 | ``` | |
306 | ||
02fe6f86 DL |
307 | Originally, tabs were used instead of spaces, with tabs are every 8 columns. |
308 | However, tab interoperability issues mean space characters are now preferred for | |
309 | new changes. We generally only clean up whitespace when code is unmaintainable | |
310 | due to whitespace issues, to minimise merging conflicts. | |
311 | ||
312 | ||
313 | #### Linux kernel & BSD style | |
314 | ||
315 | These styles are documented externally: | |
316 | ||
317 | * [https://www.kernel.org/doc/Documentation/CodingStyle](https://www.kernel.org/doc/Documentation/CodingStyle). | |
318 | * [http://man.openbsd.org/style](http://man.openbsd.org/style) | |
319 | ||
320 | They are relatively similar but differ in details. | |
321 | ||
322 | pimd deviates from Linux kernel style in using 2 spaces for indentation, with | |
323 | Tabs replacing 8 spaces, as well as adding a line break between `}` and `else`. | |
324 | It is acceptable to convert indentation in pimd/ to Linux kernel style, but | |
325 | please convert an entire file at a time. (Rationale: apart from 2-space | |
326 | indentation, the styles are sufficiently close to not upset when mixed.) | |
327 | ||
328 | Unlike GNU style, these styles use tabs, not spaces. | |
329 | ||
f6ee5b52 | 330 | |
f6ee5b52 DL |
331 | ### Compile-Time conditional code |
332 | ||
333 | Many users access PROJECT via binary packages from 3rd party sources; | |
334 | compile-time code puts inclusion/exclusion in the hands of the package | |
335 | maintainer. Please think very carefully before making code conditional at | |
336 | compile time, as it increases regression testing, maintenance burdens, and user | |
337 | confusion. In particular, please avoid gratuitous --enable-… switches to the | |
338 | configure script - typically code should be good enough to be in PROJECT, or it | |
339 | shouldn’t be there at all. | |
340 | ||
341 | When code must be compile-time conditional, try have the compiler make it | |
342 | conditional rather than the C pre-processor - so that it will still be checked | |
343 | by the compiler, even if disabled. I.e. this: | |
344 | ||
345 | ``` | |
346 | if (SOME_SYMBOL) | |
347 | frobnicate(); | |
348 | ``` | |
349 | ||
350 | rather than | |
351 | ||
352 | ``` | |
353 | #ifdef SOME_SYMBOL | |
354 | frobnicate (); | |
355 | #endif /* SOME_SYMBOL */ | |
356 | ``` | |
357 | ||
358 | Note that the former approach requires ensuring that `SOME_SYMBOL` will be | |
359 | defined (watch your `AC_DEFINE`s). | |
7ce0cb3c DS |
360 | |
361 | ### Debug-Guards in code | |
362 | ||
363 | Debugs are an important methodology to allow developers to fix issues | |
364 | found in the code after it has been released. The caveat here is | |
365 | that the developer must remember that people will be using the code | |
366 | at scale and in ways that can be unexpected for the original implementor. | |
367 | As such debugs MUST be guarded in such a way that they can be turned off. | |
368 | This PROJECT has the ability to turn on/off debugs from the CLI and it is | |
369 | expected that the developer will use this convention to allow control | |
19c7f43f DS |
370 | of their debugs. |
371 | ||
372 | ### CLI-Changes | |
373 | ||
374 | CLI's are a complicated ugly beast. Additions or changes to the CLI | |
375 | should use a DEFUN to encapsulate one setting as much as is possible. | |
376 | Additionally as new DEFUN's are added to the system, documentation | |
02fe6f86 | 377 | should be provided for the new commands. |