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: