]>
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 | ||
e0ba80e2 DS |
135 | ### Code submission - What do I submit my changes against? |
136 | ||
137 | We've documented where we would like to have the different fixes applied at | |
138 | https://github.com/FRRouting/frr/wiki/Where-Do-I-create-a-Pull-Request-against%3F | |
139 | If you are unsure where your submission goes, look at that document or ask | |
140 | the question of a maintainer. | |
f6ee5b52 | 141 | |
4765f35e | 142 | ### Code submission - Github Pull Request (Strongly Preferred) |
f6ee5b52 DL |
143 | |
144 | Preferred submission of code is by using a Github Pull Request against the | |
145 | Develop branch. Code submitted by Pull Request will have an email generated to | |
146 | the PROJECT-devel mailing list for review and the submission will be | |
147 | automatically tested by one or more CI systems. Only after this test succeeds | |
148 | (and the submission is based on the head of the develop branch), then it will | |
149 | be automatically merged into the develop branch. In case of failed tests, it is | |
150 | up to the submitter to either amend the request with further commits or close, | |
151 | fix and create a new pull request. | |
152 | ||
153 | Further (manual) code review and discussion happens after the merge into the | |
154 | develop branch. | |
155 | ||
156 | ||
4765f35e | 157 | ### Code submission - Mailing Patch to PROJECT-Devel list |
f6ee5b52 DL |
158 | |
159 | As an alternative submission, a patch can be mailed to the PROJECT-Devel | |
160 | mailing list. Preferred way to send the patch is using git send-mail. Patches | |
161 | received on the mailing list will be picked up by Patchwork and tested against | |
162 | the latest develop branch. After a further ACK by someone on the mailing list, | |
163 | the patch is then merged into the develop branch. | |
164 | ||
165 | Further (manual) code review and discussion happens after the merge into the | |
166 | develop branch. | |
167 | ||
4765f35e | 168 | #### Sending patch to mailing list |
f6ee5b52 DL |
169 | |
170 | The recommended way to send the patch (or series of NN patches) to the list is | |
171 | by using ‘git send-email’ as follows (assuming they are the most recent NN | |
172 | commit(s) in your git history: | |
173 | ||
174 | ``` | |
175 | git send-email -NN --annotate --to=XXX-Devel@XXX.org | |
176 | ``` | |
177 | ||
178 | If your commits do not already contain a `Signed-off-by` line, then use the | |
179 | following version to add it (after making sure to be able to agree to the | |
180 | Developer Certificate of Origin as outlined above): | |
181 | ||
182 | ``` | |
183 | git send-email -NN --annotate --signoff --to=XXX-Devel@XXX.org | |
184 | ``` | |
185 | ||
186 | Submitting multi-commit patches as a Github Pull Request is strongly encouraged | |
187 | and will allow your changes to merge faster | |
188 | ||
189 | ||
190 | ## After submitting your changes | |
191 | ||
192 | * Watch for Continuous Integration (CI) Test results | |
193 | * You should automatically receive an email with the test results within | |
194 | less than 2 hrs of the submission. If you don’t get the email, then check | |
195 | status on the github pull request (if submitted by pull request) or on | |
4765f35e DL |
196 | Patchwork at |
197 | [https://patchwork.PROJECT.org](https://patchwork.PROJECT.org) (if | |
198 | submitted as patch to mailing list). | |
f6ee5b52 | 199 | * Please notify PROJECT-Devel mailing list if you think something doesn’t |
4765f35e | 200 | work |
f6ee5b52 DL |
201 | * If the tests failed: |
202 | * In general, expect the community to ignore the submission until the tests | |
203 | pass. | |
204 | * It is up to you to fix and resubmit. | |
205 | * This includes fixing existing dejagnu (“make test”) tests if your | |
206 | changes broke or changed them. | |
207 | * It also includes fixing distribution packages for the failing | |
208 | platforms (ie if new libraries are required) | |
209 | * Feel free to ask for help on PROJECT-Devel list | |
210 | * Go back to the submission process and repeat until the tests pass. | |
211 | * If the tests pass: | |
212 | * If the changes are done as a pull request, then they should be | |
213 | automatically merged to the develop branch. | |
214 | * Changes sent to mailing list require a manual ACK to be merged and should | |
215 | be merged within 2 weeks. If you don’t see the merge or any | |
216 | reason/discussion on PROJECT-Devel, then please ask. | |
217 | * Watch out for questions on the mailing list. At this time there will be a | |
218 | manual code review and further (longer) tests by various community members. | |
219 | * Your submission is done once it is merged to the master branch. (which should | |
220 | happen every few weeks from the develop branch) | |
221 | ||
222 | ||
223 | ## Code Styling requirements | |
224 | ||
225 | ### File header required for new files added | |
226 | ||
4765f35e DL |
227 | New files need to have a Copyright header (see [License for |
228 | contributions](#license-for-contributions) above) added to the file. Preferred | |
229 | form of the header is as follows: | |
f6ee5b52 DL |
230 | |
231 | ``` | |
232 | /* | |
233 | Title/Function of file | |
234 | Copyright (C) 2016 Author’s Name | |
235 | ||
236 | This program is free software; you can redistribute it and/or modify | |
237 | it under the terms of the GNU General Public License as published by | |
238 | the Free Software Foundation; either version 2 of the License, or | |
239 | (at your option) any later version. | |
240 | ||
241 | This program is distributed in the hope that it will be useful, but | |
242 | WITHOUT ANY WARRANTY; without even the implied warranty of | |
243 | MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU | |
244 | General Public License for more details. | |
245 | ||
246 | You should have received a copy of the GNU General Public License | |
247 | along with this program; see the file COPYING; if not, write to the | |
248 | Free Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, | |
249 | MA 02110-1301 USA | |
250 | */ | |
4765f35e DL |
251 | |
252 | #include <zebra.h> | |
f6ee5b52 DL |
253 | ``` |
254 | ||
255 | ### Adding Copyright claims to already existing file | |
256 | ||
257 | When adding copyright claims for modifications to an existing file, please | |
258 | preface the claim with "Portions: " on a line before it and indent the | |
259 | "Copyright ..." string. If such a case already exists, add your indented claim | |
260 | immediately after. E.g.: | |
261 | ||
262 | ``` | |
263 | Portions: | |
264 | Copyright (C) 2010 Entity A .... | |
265 | Copyright (C) 2016 Your name [optional brief change description] | |
266 | ``` | |
267 | ||
268 | ### Code styling / format | |
269 | ||
02fe6f86 DL |
270 | Coding style standards in FRR vary depending on location. Pre-existing |
271 | code uses GNU coding standards. New code may use Linux kernel coding style. | |
272 | ||
273 | GNU coding style apply to the following parts: | |
274 | ||
275 | * lib/ | |
276 | * zebra/ | |
277 | * bgpd/ | |
278 | * ospfd/ | |
279 | * ospf6d/ | |
280 | * isisd/ | |
281 | * ripd/ | |
282 | * ripngd/ | |
283 | * vtysh/ | |
284 | ||
285 | Linux kernel coding style applies to: | |
286 | ||
287 | * nhrpd/ | |
288 | * watchfrr/ | |
289 | * pimd/ | |
290 | * lib/{checksum,hook,imsg-buffer,imsg,libfrr,md5,module,monotime,queue}.[ch] | |
291 | ||
292 | BSD coding style applies to: | |
293 | ||
294 | * ldpd/ | |
295 | ||
296 | **Whitespace changes in untouched parts of the code are not acceptable in | |
297 | patches that change actual code.** To change/fix formatting issues, please | |
298 | create a separate patch that only does formatting changes and nothing else. | |
299 | ||
300 | It is acceptable to rewrap entire files to Linux kernel style, but this | |
301 | **MUST** come as a separate patch that does nothing other than this | |
302 | reformatting. | |
303 | ||
304 | ||
305 | #### GNU style | |
306 | ||
307 | For GNU coding style, Indentation follows the result of invoking GNU indent: | |
f6ee5b52 DL |
308 | |
309 | ``` | |
310 | indent -nut -nfc1 file_for_submission.c | |
311 | ``` | |
312 | ||
02fe6f86 DL |
313 | Originally, tabs were used instead of spaces, with tabs are every 8 columns. |
314 | However, tab interoperability issues mean space characters are now preferred for | |
315 | new changes. We generally only clean up whitespace when code is unmaintainable | |
316 | due to whitespace issues, to minimise merging conflicts. | |
317 | ||
318 | ||
319 | #### Linux kernel & BSD style | |
320 | ||
321 | These styles are documented externally: | |
322 | ||
323 | * [https://www.kernel.org/doc/Documentation/CodingStyle](https://www.kernel.org/doc/Documentation/CodingStyle). | |
324 | * [http://man.openbsd.org/style](http://man.openbsd.org/style) | |
325 | ||
326 | They are relatively similar but differ in details. | |
327 | ||
328 | pimd deviates from Linux kernel style in using 2 spaces for indentation, with | |
329 | Tabs replacing 8 spaces, as well as adding a line break between `}` and `else`. | |
330 | It is acceptable to convert indentation in pimd/ to Linux kernel style, but | |
331 | please convert an entire file at a time. (Rationale: apart from 2-space | |
332 | indentation, the styles are sufficiently close to not upset when mixed.) | |
333 | ||
334 | Unlike GNU style, these styles use tabs, not spaces. | |
335 | ||
f6ee5b52 | 336 | |
f6ee5b52 DL |
337 | ### Compile-Time conditional code |
338 | ||
339 | Many users access PROJECT via binary packages from 3rd party sources; | |
340 | compile-time code puts inclusion/exclusion in the hands of the package | |
341 | maintainer. Please think very carefully before making code conditional at | |
342 | compile time, as it increases regression testing, maintenance burdens, and user | |
343 | confusion. In particular, please avoid gratuitous --enable-… switches to the | |
344 | configure script - typically code should be good enough to be in PROJECT, or it | |
345 | shouldn’t be there at all. | |
346 | ||
347 | When code must be compile-time conditional, try have the compiler make it | |
348 | conditional rather than the C pre-processor - so that it will still be checked | |
349 | by the compiler, even if disabled. I.e. this: | |
350 | ||
351 | ``` | |
352 | if (SOME_SYMBOL) | |
353 | frobnicate(); | |
354 | ``` | |
355 | ||
356 | rather than | |
357 | ||
358 | ``` | |
359 | #ifdef SOME_SYMBOL | |
360 | frobnicate (); | |
361 | #endif /* SOME_SYMBOL */ | |
362 | ``` | |
363 | ||
364 | Note that the former approach requires ensuring that `SOME_SYMBOL` will be | |
365 | defined (watch your `AC_DEFINE`s). | |
7ce0cb3c DS |
366 | |
367 | ### Debug-Guards in code | |
368 | ||
369 | Debugs are an important methodology to allow developers to fix issues | |
370 | found in the code after it has been released. The caveat here is | |
371 | that the developer must remember that people will be using the code | |
372 | at scale and in ways that can be unexpected for the original implementor. | |
373 | As such debugs MUST be guarded in such a way that they can be turned off. | |
374 | This PROJECT has the ability to turn on/off debugs from the CLI and it is | |
375 | expected that the developer will use this convention to allow control | |
19c7f43f DS |
376 | of their debugs. |
377 | ||
378 | ### CLI-Changes | |
379 | ||
380 | CLI's are a complicated ugly beast. Additions or changes to the CLI | |
381 | should use a DEFUN to encapsulate one setting as much as is possible. | |
382 | Additionally as new DEFUN's are added to the system, documentation | |
02fe6f86 | 383 | should be provided for the new commands. |