]>
Commit | Line | Data |
---|---|---|
dd23902d | 1 | # ISC DHCP Contributor's Guide |
6ee76860 | 2 | |
dd23902d TM |
3 | So you found a bug in ISC DHCP or plan to develop an extension and want to send us a patch? Great! |
4 | This page will explain how to contribute your changes smoothly. | |
6ee76860 | 5 | |
0cd94b5e TM |
6 | We do not require a contributors agreement. By submitting a patch or merge request to this project, |
7 | you are agreeing that your code will be covered by the primary license for the project. | |
8 | ISC DHCP is currently licensed under the MPL2.0 license. | |
9 | ||
10 | Here's are the steps in contributing a patch: | |
11 | ||
12 | 1. **create account** on [gitlab](https://gitlab.isc.org) | |
13 | 2. **open an issue** in [this project](https://gitlab.isc.org/isc-projects/dhcp/issues/new), make sure | |
14 | it describes what you want to fix and **why**. ISC DHCP is very mature code, with a large installed base. | |
15 | We are fairly conservative about making changes unless there is a very good reason. | |
16 | 3. **ask someone from the ISC team to give you a 'project allocation' so you can to fork ISC DHCP in our repo** (ask on the issue - mention @tomek, @vicky, @ondrej | |
17 | or @godfryd if it seems we haven't noticed your request) | |
18 | 4. **fork the DHCP master branch**: go to the DHCP project page, click the [Fork button](https://gitlab.isc.org/isc-projects/dhcp/forks/new). | |
19 | If you can't, you didn't complete step 3. It helps to include the issue number and subject in the branch name. | |
20 | 5. **Implement your fix or feature, in your branch**. Make sure it compiles, has unit-tests, | |
21 | is documented and does what it's supposed to do. | |
22 | 6. **Open Merge Request**: go to the DHCP project [merge requests page](https://gitlab.isc.org/isc-projects/dhcp/merge_requests), and | |
23 | click [New merge request](https://gitlab.isc.org/isc-projects/dhcp/merge_requests/new). If you | |
24 | don't see the button, you didn't complete step 3. | |
25 | 7. **Participate in the code review**: Once you submit the MR, someone from ISC will eventually get | |
26 | to the issue and will review your code. Please make sure you respond to comments. It's likely | |
27 | you'll be asked to update the code. | |
28 | ||
29 | See the text below for more details. | |
30 | ||
31 | ||
32 | ## Create an issue | |
33 | ||
34 | The first step in contributing to ISC DHCP is to [create an issue](https://gitlab.isc.org/isc-projects/dhcp/issues/new), describing the problem, deficiency | |
35 | or missing feature you want to address. It is important to make it very clear why the specific change | |
36 | you are proposing should be made. ISC DHCP is very mature code, with a large and somewhat inert installed base. | |
37 | We are very cautious about introducing changes that could break existing functionalty. If you want to fix | |
38 | multiple problems, or make multiple changes, please make separate issues for each. | |
39 | ||
40 | ## Plan your changes | |
6ee76860 | 41 | |
228aa403 | 42 | Before you start working on a patch or a new feature, it is a good idea to discuss it first with |
0cd94b5e TM |
43 | DHCP developers. You may benefit from reading the [ISC DHCP Developer's Survival Guide](https://gitlab.isc.org/isc-projects/dhcp/wikis/home) |
44 | posted on the wiki page for this repo. | |
45 | ||
46 | You can post questions about development on the [dhcp-workers](https://lists.isc.org/mailman/listinfo/dhcp-workers) | |
47 | or [dhcp-users](https://lists.isc.org/mailman/listinfo/dhcp-users) mailing lists. Dhcp-users is | |
48 | intended for users who are not interested in development details: it is appropriate to ask for | |
49 | feedback regarding the best proposed solution to a certain problem. The internal details, | |
50 | questions about the code and its internals are better asked on dhcp-workers. The dhcp-workers | |
51 | list is a very low traffic list. | |
52 | ||
53 | ||
54 | ## Create a branch for your work | |
55 | ||
56 | These instructions assume you will be making your changes on a branch in the ISC DHCP Gitlab | |
57 | repository. This is by far the easiest way for us to collaborate with you. While we also maintain a presence | |
58 | on [Github](https://github.com/isc-projects/dhcp), ISC developers rarely look at Github, which is | |
59 | just a mirror of our Gitlab system. | |
60 | ||
61 | ISC's Gitlab has been a target for spammers, so it is set up defensively. New users need permission | |
62 | from ISC to create new projects. We gladly do this for anyone who asks and provides a good reason. | |
63 | "I'd like to fix bug X or develop feature Y" is an excellent reason. To request a project | |
64 | allocation in ISC's Gitlab, just ask for it in a comment in your issue. Make sure | |
65 | you tag someone at ISC (@tomek, @godfryd, @vicky or @ondrej). When you write a comment in an issue or | |
66 | merge request and add a name tag on it, the user is automatically notified. | |
67 | ||
68 | Once you are given a 'project allocation' in our Gitlab, you can fork ISC DHCP and create a branch. | |
69 | This is your copy of ISC DHCP and is where you will make your changes. Go to the DHCP project page, | |
70 | click the [Fork button](https://gitlab.isc.org/isc-projects/dhcp/forks/new) and you will be prompted | |
71 | to name your branch. It helps to include the issue number and subject in the branch name. You can make | |
72 | changes to this branch without worrying that you will impact the master branch - commit priviliges | |
73 | are restricted so you cannot accidentally alter the master branch. | |
74 | ||
75 | Please read the [Gitlab How-To](https://gitlab.isc.org/isc-projects/dhcp/wikis/processes/gitlab-howto) for ISC DHCP. | |
76 | ||
77 | ||
78 | ## Implement your change | |
79 | ||
80 | Please try to conform to the project's coding standards. ISC DHCP uses the same [coding standards](https://gitlab.isc.org/isc-projects/bind9/blob/master/doc/dev/style.md) as the BIND 9 project. https://gitlab.isc.org/isc-projects/bind9/blob/master/doc/dev/style.md | |
81 | ||
82 | ||
83 | ## Compile your code | |
84 | ||
85 | We don't yet have continuous integration set up for ISC DHCP, so you have to check the compilation manually. | |
86 | ISC DHCP is used on a wide array of UNIX and Linux operating systems. Will your code compile and work there? | |
87 | What about endianness? It is likely that you used a regular x86 architecture machine to write your | |
88 | patch, but the software is expected to run on many other architectures. . | |
89 | ||
90 | ## Run unit-tests | |
6ee76860 | 91 | |
228aa403 | 92 | One of the ground rules in all ISC projects is that every piece of code has to be tested. For newer |
0cd94b5e | 93 | projects, we require a unit-test for almost every line of code. For older code, such as |
228aa403 TM |
94 | ISC DHCP, that was not developed with testability in mind, it's unfortunately impractical to require |
95 | extensive unit-tests. Having said that, please think thoroughly if there is any way to develop | |
96 | unit-tests. The long term goal is to improve the situation. | |
97 | ||
0cd94b5e TM |
98 | Where unit tests are not practical, supplying us with things like configuration file(s), lease file(s), |
99 | PCAPS, and step-by-step on how you tested the changes would be a big help. This will aid us in | |
100 | creating and adding system tests to the build farm. | |
101 | ||
dd23902d TM |
102 | You should have also conducted some sort of system testing to verify that your changes do what you |
103 | want. It would be extremely helpful if you can attach any configuration files (dhcpd and or | |
104 | dhclient), along with a step-by-step procedure to carry out the test(s). This will help us verify | |
105 | your changes as extend our own system tests. | |
106 | ||
dd23902d TM |
107 | Make sure you have ATF (Automated Test Framework) installed in your system. For more information |
108 | about ATF, please refer to <dhcp source tree>/doc/devel/atf.dox. Note, running "make devel" in this | |
109 | directory will generate the documentation. To run the unit-tests, simply run: | |
228aa403 TM |
110 | |
111 | ```bash | |
0cd94b5e TM |
112 | ./configure --with-atf |
113 | make | |
228aa403 TM |
114 | make check |
115 | ``` | |
116 | ||
117 | If you happen to add new files or have modified any Makefile.am files, it is also a good idea to | |
118 | check if you haven't broken the distribution process: | |
6ee76860 TM |
119 | |
120 | ```bash | |
121 | make distcheck | |
122 | ``` | |
123 | ||
228aa403 TM |
124 | There are other useful switches which can be passed to configure. A complete list of all switches |
125 | can be obtained with the command: | |
6ee76860 TM |
126 | |
127 | ```bash | |
128 | ./configure --help | |
129 | ``` | |
130 | ||
0cd94b5e | 131 | ## Create a Merge Request |
228aa403 | 132 | |
0cd94b5e | 133 | Once you feel that your patch is ready, go to the DHCP project |
dd23902d | 134 | and [submit a Merge Request](https://gitlab.isc.org/isc-projects/dhcp/merge_requests/new). |
6ee76860 | 135 | |
0cd94b5e TM |
136 | If you can't access this link or don't see New Merge Request button on the [merge requests |
137 | page](https://gitlab.isc.org/isc-projects/dhcp/merge_requests), please ask on dhcp-workers and someone | |
138 | will help you out. | |
6ee76860 | 139 | |
228aa403 TM |
140 | Once you submit it, someone from the DHCP development team will look at it and will get back to you. |
141 | The dev team is very small, so it may take a while... | |
6ee76860 | 142 | |
0cd94b5e | 143 | ## If you really can't do a merge request on ISC's Gitlab... |
6ee76860 | 144 | |
228aa403 TM |
145 | Well, you are out of luck. There are other ways, but those are really awkward and the chances of |
146 | your patch being ignored are really high. Anyway, here they are: | |
147 | ||
148 | - Create a ticket in the DHCP Gitlab (https://gitlab.isc.org/isc-projects/dhcp) and attach your | |
149 | patch to it. Sending a patch has a number of disadvantages. First, if you don't specify the base | |
150 | version against which it was created, one of ISC engineers will have to guess that or go through | |
151 | a series of trials and errors to find that out. If the code doesn't compile, the reviewer will not | |
152 | know if the patch is broken or maybe it was applied to incorrect base code. Another frequent | |
153 | problem is that it may be possible that the patch didn't include any new files you have added. | |
154 | ||
155 | - Send a patch to the dhcp-workers list. This is even worse, but still better than not getting the | |
156 | patch at all. The problem with this approach is that we don't know which version the patch was | |
157 | created against and there is no way to track it. So the chances of it being forgotten are high. | |
158 | Once a DHCP developer get to it, the first thing he/she will have to do is try to apply your | |
159 | patch, create a branch commit your changes and then open MR for it. | |
160 | ||
161 | ## Going through a review | |
6ee76860 | 162 | |
0cd94b5e TM |
163 | Once the merge request (MR) is in the system, the action is on one of the core developers. |
164 | ||
165 | Sooner or later, one of these engineers will do the review. Unfortunately, we have a small team | |
166 | and we have a lot of users to support, so it may take a while for us to get to your patch. | |
167 | Having said that, we value external contributions very much and will do whatever we | |
168 | can to review patches in a timely manner. | |
228aa403 | 169 | |
0cd94b5e TM |
170 | Don't get discouraged if your patch is not accepted on the first review. To keep the code |
171 | quality high, we use the same review processes for external patches as we do for internal code. | |
172 | It may take some cycles of review/updated patch submissions before the code is finally accepted. | |
173 | The nature of the review process is that it emphasizes areas that need improvement. If you are | |
174 | not used to the review process, you may get the impression that the feedback is negative. It | |
175 | is not: even the core developers seldom see reviews that say "All OK please merge". | |
228aa403 TM |
176 | |
177 | If we happen to have any comments that you as submitter are expected to address (and in the | |
0cd94b5e TM |
178 | overwhelming majority of cases, we have), you will be asked to update your MR. It is common |
179 | to see several rounds of such reviews. | |
228aa403 TM |
180 | |
181 | Once the process is almost complete, the developer will likely ask you how you would like to be | |
182 | credited. The typical answers are by first and last name, by nickname, by company name or | |
183 | anonymously. Typically we will add a note to the ChangeLog and also set you as the author of the | |
184 | commit applying the patch and update the contributors section in the AUTHORS file. If the | |
185 | contributed feature is big or critical for whatever reason, it may also be mentioned in release | |
186 | notes. | |
187 | ||
188 | Sadly, we sometimes see patches that are submitted and then the submitter never responds to our | |
189 | comments or requests for an updated patch. Depending on the nature of the patch, we may either fix | |
0cd94b5e | 190 | the outstanding issues on our own and get another engineer to review them or the ticket may end |
228aa403 TM |
191 | up in our Outstanding milestone. When a new release is started, we go through the tickets in |
192 | Outstanding, select a small number of them and move them to whatever the current milestone is. Keep | |
193 | that in mind if you plan to submit a patch and forget about it. We may accept it eventually, but | |
0cd94b5e | 194 | it's a much, much faster process if you participate in it. |
228aa403 | 195 | |
0cd94b5e | 196 | #### Thank you for contributing your time and expertise to the ISC DHCP Project. |