1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157
|
This is a guide to submitting patches to the concordance project.
TABLE OF CONTENTS
1. Preparing a change
2. Generating patches
3. Preparing and submitting your patch
4. Additional notes for major changes
5. A word on comments
PREPARING A CHANGE
If you would like to make a change to the codebase, you need to decide if this
is a major change or a minor change. Major changes include anything that
affects the API, ABI, anything that affects lots of files, adds/removes
classes, etc.
If your plan is to make a major change, then you should send a description of
the changes you plan to make to the concordance-devel list FIRST. By doing
this you give the developers a chance to comment on your plan and provide
suggestions and guidance before you invest the time. This will save you time
and effort and provide a much higher chance of your patch being accepted. Also
see "ADDITIONAL NOTES FOR MAJOR CHANGES" below.
If your plan is to make a minor change, you are still welcome to send an email
before you start to the list, but it's not as big of a deal.
GENERATING PATCHES
After making your changes, you should always make sure you have done a 'cvs
update' to ensure you'll be generating your patch against the latest CVS
sources.
Patches should always be generated via CVS's diff, and from top-level CVS
directory. Additionally, they should always be in unified diff format ("-u"),
should show new/removed files ("-N"), be recursive ("-R"), and use the -p flag
to make sure the C function name is included for all changes. For example:
CVSTREE=~/cvs/concordance
cd $CVSTREE
vi libconcord/remote.cpp
cvs diff -NRup >/tmp/libconcord_remote.patch
You should ALWAYS read your patch through after generating it to make sure it
contains the changes you expect and ONLY the changes you expect.
You should ensure the following:
1. You've maintained the same coding style. See the CodingStyle document
in this directory.
2. You should make sure that your code is properly commented.
PREPARING AND SUBMITTING YOUR PATCH
Your patch should be sent in as an attachement. Mail clients often wrap emails
(and rightly so!) which can break a patch. Attach your patch to your email.
The body of your email should have a good descriptiong of your patch. The more
far-reaching your patch is, the more detailed your description should be.
A good description includes justifying any ABI/API changes, an explanation of
the goal and outcome of the patch, and a list of changes by file, where
appropriate.
The subject of an email SHOULD start with "PATCH: " and include a _useful_
description of your patch. Where possible, include the component affected
(concordance vs. libconcord) GOOD examples include:
PATCH: concordance: fix double-free bug
PATCH: libconcord: Add remote_info table entry for 4847 byte location
BAD examples include:
PATCH: update
PATCH: libconcord fix
PATCH: new remote
NOTE: PATCHES SHOULD ALWAYS BE SENT TO THE -devel LIST! Even if you are
attaching a patch to a bug, you should still send it to the devel list if you
want it committed.
PATCHSET SUBMISSIONS
Sometimes it makes sense to break a given change into several pieces, but you
still want to submit them together. In this case, you may do this in two ways.
You could just you attach all patches to a single email with useful filenames
like 1_update_foo_API.patch 2_update_bar_API and a list of the patches and
their details in the email.
Alternatively, if you prefer multiple emails, you can send a group of emails
as follows.
An initial email with a subject that starts with "[PATCH 0/X]" (where X is the
total number of patches) and a short description should be sent. This email
should contain all the information about the patchset as a whole, as well as a
list of patch-containing-emails that will follow. This list should look like
this:
1/2 - libconcord: update foo() API to accept new parameter
2/2 - concordance: use new foo() paremeter to do bar
Assuming here that X is 2. Then these patches should be submitted with
subjects like: "[PATCH 1/2] libconcord: update foo() API to accept new param."
Note that if there's a possiblity of merging some parts of your patchset in
stages, then they should definitely be split into multiple emails.
ADDITIONAL NOTES FOR MAJOR CHANGES
As previously mentioned, all major changes should be mentioned on the list.
Note, however, that changes to the ABI or API need to be justified. The API,
in particular, should change as little as possible.
If a change to the API is needed, remember that the API is *C* API even though
the library itself is written in C++. Any changes to the API must ensure it's
still C-compatible, and that all of the API is exported properly for C
clients.
In addition, all API changes must be added to all bindings we support as well.
If you don't have experience with some of the languages, we can help.
A WORD ON COMMENTS
Comments should be immediately above the block of code they are explaining,
NOT next to it. For example, we prefer this:
/*
* This special algorith for XXX does A, then B, then C, then D.
* It is necessary because of bar.
*/
if (foo) {
...
}
Not this:
if (foo) {
a = *(b+8); // Some comment here
In addition, comments should explain code, not just narrate it. A comments
that says "skip the first byte" isn't useful, but a comment that says "the
first byte is all 0 for Windows, so we skip it when not in Windows" is very
useful.
# for vim to text-wrap
vim:textwidth=78:
|