|
@@ -9,7 +9,7 @@
|
|
|
@page contributorGuide Kea Contributor's Guide
|
|
|
|
|
|
So you found a bug in Kea or plan to develop an extension and want to
|
|
|
-send a patch? Great! This page will explain how to contribute your
|
|
|
+send us a patch? Great! This page will explain how to contribute your
|
|
|
changes smoothly.
|
|
|
|
|
|
@section contributorGuideWritePatch Writing a patch
|
|
@@ -25,14 +25,6 @@ design or the best proposed solution to a certain problem, but all
|
|
|
the internal details should be discussed on kea-dev and not posted
|
|
|
to kea-users.
|
|
|
|
|
|
-If you prefer to get
|
|
|
-faster feedback, most Kea developers hang out in the \c dhcp
|
|
|
-jabber room (xmpp:dhcp@conference.jabber.isc.org). Feel free to join this
|
|
|
-room and talk to us. It is possible that someone else is working on your
|
|
|
-specific issue or perhaps the solution you plan to implement is not
|
|
|
-the best one. Often having a 10 minute talk could save many hours of
|
|
|
-engineering work.
|
|
|
-
|
|
|
The first step in writing the patch or new feature should be to get
|
|
|
the source code from our Git repository. The procedure is very easy and
|
|
|
is explained here: http://kea.isc.org/wiki/GitGuidelines. While it is
|
|
@@ -44,18 +36,18 @@ OK, so you have written a patch? Great! Before you submit it, make sure
|
|
|
that your code compiles. This may seem obvious, but there's more to
|
|
|
it. You have surely checked that it compiles on your system, but Kea
|
|
|
is portable software. Besides Linux, it is compiled and used on
|
|
|
-relatively uncommon systems like OpenBSD and Solaris 11. Will your code
|
|
|
+relatively uncommon systems like OpenBSD. Will your code
|
|
|
compile and work there? What about endianess? It is likely that you used
|
|
|
a regular x86 architecture machine to write your patch, but the software
|
|
|
is expected to run on many other architectures. You may take a look at
|
|
|
-system specific build notes (http://kea.isc.org/wiki/SystemSpecificNotes).
|
|
|
+system specific build notes (http://kea.isc.org/wiki/Install).
|
|
|
For a complete list of systems we build on, you may take a look at the
|
|
|
following build farm report: https://jenkins.isc.org/view/Kea_BuildFarm/ .
|
|
|
|
|
|
Does your patch conform to Kea coding guidelines
|
|
|
(http://kea.isc.org/wiki/CodingGuidelines)? You can submit a
|
|
|
patch that does not adhere to them, but that will reduce its chances of
|
|
|
-being accepted. If the deviations are minor, the Kea engineer who
|
|
|
+being accepted. If the deviations are minor, one of the Kea engineers who
|
|
|
does the review will likely fix the issues. However, if there are lots
|
|
|
of issues, the reviewer may simply reject the patch and ask you to fix
|
|
|
it before re-submitting.
|
|
@@ -94,7 +86,24 @@ written and observing the test fail, you can detect the situation
|
|
|
where a bug in the test code will cause it to pass regardless of
|
|
|
the code being tested.
|
|
|
|
|
|
-See @ref unitTests for instructions on how to run unit-tests.
|
|
|
+Initially, some people unfamiliar with that approach react with "but
|
|
|
+my change is simple and I tested that it works". That approach
|
|
|
+is both insufficient and short-sighted. It is insufficient, because
|
|
|
+manual testing is by definition laborious and can't really be done
|
|
|
+on the multitude of systems we run Kea on. It is short-sighted, because
|
|
|
+even with your best intentions you will not be able to dedicate
|
|
|
+any significant amount of time for repeated testing of your improved
|
|
|
+code. The old ISC DHCP has two decades of history behind it and we
|
|
|
+hope to make Kea last similar time span. Over such long periods,
|
|
|
+code tends to be refactored several times. The change you
|
|
|
+made may be affected by some other change or by the code that
|
|
|
+hasn't been written yet.
|
|
|
+
|
|
|
+See @ref unitTests for instructions on how to run unit-tests. If you
|
|
|
+happen to touch any database related code, make sure you compile
|
|
|
+your code with @c --with-dhcp-mysql, @c --with-dhcp-pgsql and/or
|
|
|
+@c --with-cql as needed. For example, if you change something
|
|
|
+substantial, make sure the other compilation options still work.
|
|
|
|
|
|
If you happen to add new files or have modified any \c Makefile.am
|
|
|
files, it is also a good idea to check if you haven't broken the
|
|
@@ -120,47 +129,125 @@ A complete list of all switches can be obtained with the command:
|
|
|
./configure --help
|
|
|
@endcode
|
|
|
|
|
|
+@section contributorGuideSendPatch Sending the patch
|
|
|
+
|
|
|
+There are several ways how you can send your changes. They are listed
|
|
|
+here from most to least preferred.
|
|
|
+
|
|
|
+- Send pull request (PR) on github. This is by far the most convenient way for
|
|
|
+everyone. See the excellent documentation on github:
|
|
|
+https://help.github.com/articles/creating-a-pull-request/ for details. In
|
|
|
+essence, you need github account (spam/hassle free, takes one minute to set
|
|
|
+up). Then you can fork the Kea repository, commit changes to your repo and ask us to pull
|
|
|
+your changes into official Kea repository. This has a number of advantages. First, it is
|
|
|
+made against a specific code version, which can be easily checked with git log
|
|
|
+command. Second, this request pops up instantly on our list of open pull
|
|
|
+requests and will stay there. The third benefit is that the pull request mechanism is
|
|
|
+very flexible. Kea engineers (and other users, too) can comment on it, attach
|
|
|
+links, mention other users etc. You as a submitter can augment the patch by
|
|
|
+commiting extra changes to your repository. Those extra commits will appear instantly
|
|
|
+in the pull request. This is really useful during the review. Finally, ISC engineers can
|
|
|
+better assess all open pull requests and add labels to them, such as "enhancement", "bug", or
|
|
|
+"unit-tests missing". This makes our life easier. Oh, and your commits will later
|
|
|
+be shown as yours in github history. If you care for that kind of things, once the
|
|
|
+patch is merged, you'll be automatically listed as contributor and Kea will be
|
|
|
+listed as project you have contributed to.
|
|
|
+
|
|
|
+- Create a ticket in the Kea trac and attach your patch to it. Sending a patch has a
|
|
|
+number of disadvantages. First, if you don't specify the base version against
|
|
|
+which it was created, one of ISC engineers will have to guess that or go through
|
|
|
+a series of trials and errors to find that out. If the code doesn't compile, the
|
|
|
+reviewer will not know if the patch is broken or maybe it was applied to
|
|
|
+incorrect base code. Another frequent problem is that it may be possible that
|
|
|
+the patch didn't include any new files you have added. If we happen to have any
|
|
|
+comments that you as submitter are expected to address (and in the overwhelming majority
|
|
|
+of cases, we have), you will be asked to send an updated patch. It is not
|
|
|
+uncommon to see several rounds of such reviews, so this can get very complicated
|
|
|
+very quickly. Please make sure your ticket is created in the default milestone
|
|
|
+"kea-proposed". ISC engineers review new tickets once a week and assign them to
|
|
|
+specific milestones. Please do not add tickets to working milestones directly.
|
|
|
+Having a ticket in trac ensures that the patch will never be forgotten and it
|
|
|
+will show up on our trac reports. It's not required, but much appreciated if
|
|
|
+you send a short note to the kea-dev mailing list explaining what you did with the code
|
|
|
+and announce the ticket number.
|
|
|
+
|
|
|
+- Send a patch to the kea-dev list. This is the third preferred method, if you
|
|
|
+can't or don't want to use github and trac for whatever reason. If you
|
|
|
+send a patch to a mailing list in a wrong time, e.g. shortly before a release,
|
|
|
+ISC engineers may miss it or perhaps they will see it and then forget about
|
|
|
+it. Nevertheless, it is still doable and we successfully accepted patches that
|
|
|
+way. It just takes more time from everyone involved, so it's a slower process in
|
|
|
+general.
|
|
|
+
|
|
|
+- Send a tarball with your modified code. This is really the worst way one
|
|
|
+can contribute a patch. It has a number of disadvantages. In particular, someone
|
|
|
+will need to find out which version the code was based on and generate the
|
|
|
+patch. It's not a rocket science, but it may be a very mundane thing to do
|
|
|
+if the ISC engineer does not know the version in advance. The mailing list
|
|
|
+has a limit on the message size (for good reasons), so you'll likely need to
|
|
|
+upload it somewhere first. ISC engineers often don't pick up new tickets
|
|
|
+instantly, so it may have to wait weeks before the tarball is looked at.
|
|
|
+The tarball does not benefit from most of the advantages mentioned for github,
|
|
|
+like the ability to easily update the code, have a meaningful discussion
|
|
|
+or see what the exact scope of changes are. Nevertheless, if we given a choice
|
|
|
+of getting a tarball or not getting a patch at all, we prefer tarballs. Just
|
|
|
+keep in mind that processing a tarball is really cumbersome for ISC
|
|
|
+engineers, so it may take sigificantly longer than other ways.
|
|
|
+
|
|
|
@section contributorGuideReview Going through a review
|
|
|
|
|
|
-Once everything is checked and working, feel free to create a ticket for
|
|
|
-your patch at http://kea.isc.org/ or attach your patch to an existing
|
|
|
-ticket if you have fixed it. It would be nice if you also join the
|
|
|
-\c dhcp chatroom saying that you have submitted a patch. Alternatively,
|
|
|
-you may send a note to the \c kea-dev mailing list.
|
|
|
-
|
|
|
-Here's the tricky part. One of Kea developers will review your patch,
|
|
|
-but it may not happen immediately. Unfortunately, developers are usually
|
|
|
-working under a tight schedule, so any extra unplanned review work may
|
|
|
-take a while sometimes. Having said that, we value external
|
|
|
-contributions very much and will do whatever we can to review patches in
|
|
|
-a timely manner. Don't get discouraged if your patch is not accepted
|
|
|
-after first review. To keep the code quality high, we use the same
|
|
|
+Once you let us submitted a patch using one of the ways above, the action
|
|
|
+is on one of the ISC engineers. First, we will need either a trac ticket
|
|
|
+or PR on github. We prefer the original submitter fill them as he or she
|
|
|
+has the best understanding of the purpose of the change and may have
|
|
|
+any extra information, e.g. "this patch fixes compilation issue on
|
|
|
+FreeBSD 10.1". If there there is no PR and no trac ticket, we will create
|
|
|
+one. Depending on the subjective importance and urgency as perceived by
|
|
|
+the ISC engineer, the ticket or PR will be assigned to one of the
|
|
|
+milestones.
|
|
|
+
|
|
|
+Sooner or later, one of ISC engineers will do the review. Here's the tricky
|
|
|
+part. One of Kea developers will review your patch, but it may not happen
|
|
|
+immediately. Unfortunately, developers are usually working under a tight
|
|
|
+schedule, so any extra unplanned review work may take a while sometimes. Having
|
|
|
+said that, we value external contributions very much and will do whatever we can
|
|
|
+to review patches in a timely manner. Don't get discouraged if your patch is not
|
|
|
+accepted after first review. To keep the code quality high, we use the same
|
|
|
review processes for external patches as we do for internal code. It may take
|
|
|
-some cycles of review/updated patch submissions before the code is
|
|
|
-finally accepted. The nature of the review process is that it emphasizes
|
|
|
-areas that need improvement. If you are not used to the review process,
|
|
|
-you may get the impression that the feedback is negative. It is not: even
|
|
|
-the Kea developers seldom see reviews that say "All OK please merge".
|
|
|
-
|
|
|
-Once the process is almost complete, the developer will likely ask you
|
|
|
-how you would like to be credited. The typical answers are by first and
|
|
|
-last name, by nickname, by company name or anonymously. Typically we
|
|
|
-will add a note to the \c ChangeLog and also set you as the author of
|
|
|
-the commit applying the patch. If the contributed feature is big or
|
|
|
-critical for whatever reason, it may also be mentioned in release notes.
|
|
|
+some cycles of review/updated patch submissions before the code is finally
|
|
|
+accepted. The nature of the review process is that it emphasizes areas that need
|
|
|
+improvement. If you are not used to the review process, you may get the
|
|
|
+impression that the feedback is negative. It is not: even the Kea developers
|
|
|
+seldom see reviews that say "All OK please merge".
|
|
|
+
|
|
|
+Once the process is almost complete, the developer will likely ask you how you
|
|
|
+would like to be credited. The typical answers are by first and last name, by
|
|
|
+nickname, by company name or anonymously. Typically we will add a note to the \c
|
|
|
+ChangeLog and also set you as the author of the commit applying the patch and
|
|
|
+update the contributors section in the AUTHORS file. If the contributed feature
|
|
|
+is big or critical for whatever reason, it may also be mentioned in release
|
|
|
+notes.
|
|
|
+
|
|
|
+Sadly, we sometimes see patches that are submitted and then the submitter never
|
|
|
+responds to our comments or requests for an updated patch. Depending on the
|
|
|
+nature of the patch, we may either fix the outstanding issues on our own and get
|
|
|
+another ISC engineer to review them or the ticket may end up in our "Outstanding
|
|
|
+Tasks" milestone. When a new release is started, we go through the tickets in
|
|
|
+Outstanding Tasks, select a small number of them and move them to whatever the
|
|
|
+current milestone is. Keep that in mind if you plan to submit a patch and forget
|
|
|
+about it. We may accept it eventually, but it's much, much faster process if you
|
|
|
+participate in it.
|
|
|
|
|
|
@section contributorGuideExtra Extra steps
|
|
|
|
|
|
-If you are interested in knowing the results of more in-depth testing,
|
|
|
-you are welcome to visit the ISC Jenkins page: https://jenkins.isc.org
|
|
|
-(Our old Kea build farm http://git.kea.isc.org/~tester/builder/KEA-builder-new.html
|
|
|
-is being migrated to Jenkins). This is a
|
|
|
-live result page with all tests being run on various systems. Besides
|
|
|
-basic unit-tests, we also have reports from valgrind (memory debugger),
|
|
|
-cppcheck and clang-analyzer (static code analyzers), Lettuce system
|
|
|
-tests and more. Although it is not possible for non ISC employees to run
|
|
|
-tests on that farm, it is possible that your contributed patch will end
|
|
|
-up there sooner or later. We also have ISC Forge tests running, but currently
|
|
|
-the test results are not publicly available.
|
|
|
+If you are interested in knowing the results of more in-depth testing, you are
|
|
|
+welcome to visit the ISC Jenkins page: https://jenkins.isc.org This is a live
|
|
|
+result page with all tests being run on various systems. Besides basic
|
|
|
+unit-tests, we also have reports from valgrind (memory debugger), cppcheck and
|
|
|
+clang-analyzer (static code analyzers), Lettuce system tests and more. Although
|
|
|
+it is not possible for non ISC employees to run tests on that farm, it is
|
|
|
+possible that your contributed patch will end up there sooner or later. We also
|
|
|
+have ISC Forge tests running, but currently the test results are not publicly
|
|
|
+available.
|
|
|
|
|
|
*/
|