Beyond looking for changes that improve Xapian, code that works
and so forth, there are a number of things that we aim for when
accepting changes. This then is a list of good practices when
contributing changes.

We like Xapian to compile without any warnings. In “maintainer
mode”, which will be how you’re building Xapian if you’re
working from a git clone, all warnings will actually become
errors. You should fix the problems rather than change the
compilation settings to ignore these warnings.

We don’t currently have a formal coding standards document, so
you should try to follow the style of the existing
code. In particular, it’s a good idea to pay close attention
to code alignment and where we have spaces.

If you add a new feature, please ensure that you’ve documented
it. Don’t worry too much about the language you use, or if
English isn’t your first language. Others can help get the
documentation into shape, but having a first draft from the
person who wrote the feature is usually the best way to get
started.

API classes, methods, functions, and types should be
documented by documentation comments alongside the
declaration in include/xapian/*.h.

These are collated by doxygen – see doxygen’s documentation
for details of the supported syntax. We’ve decided to prefer
to use @ rather than \ to introduce doxygen commands
(the choice is essentially arbitrary, but \ introduces
C/C++ escape sequences so @ is likely to make for easier
to read mark up for C/C++ coders).

The documentation comments don’t give users a good overview,
so we also need documentation which gives a good overview of
how to achieve particular tasks.

If there’s relevant documentation already in the user guide,
then you should update that. For completely new features,
you should create either a “how to” or an “advanced feature”
document in the user manual, so that people can get started
without having to start with the API documentation.

Internal classes, etc should also be documented by
documentation comments where they are declared.

If you’re fixing a bug, you should first write a regression
test. The test will fail on the existing code, then when you
fix the bug it will pass. In the future, the test will make
sure no one accidentally re-introduces the same bug.

If you’re adding a new feature, you’ll want to write tests that
it behaves correctly. Thinking about the tests you need to
write can often help you plan how to implement the feature; it
can also help when thinking about what API any new classes or
methods should expose.

If necessary, modify the copyright statement at the top of any
files you’ve altered. If there is no copyright statement, you may
add one (there are a couple of Makefile.am’s and similar that
don’t have copyright statements; anything that small doesn’t
really need one anyway, so it’s a judgement call). If you’ve
added files which you’ve written from scratch, they should
include the GPL boilerplate with your name only.

If you’re not in there already, add yourself to the
xapian-core/AUTHORS file.

If there’s an active release branch, please check if the bug is present
in that branch, and if the fix is appropriate to backport - if the fix
breaks ABI compatibility or is very invasive, you may need to fix it in
a different way for the release branch, or decide not to backport the fix.

We ask everyone contributing changes to Xapian to dual-license under the GPL (which Xapian currently uses) and the MIT/X
license (which we would like to move to in future). The simplest way
to do this is to drop an email to the xapian-devel mailing list stating that you own the copyright on your
changes and are happy to dual-license accordingly.

There are two ways of working, depending on whether you want to use
Github or not. In both cases, review and acceptance of the changes
will generally go more easily if you’ve included tests, updated
documentation and so on as discussed earlier.

We find patches in unified diff format easiest to work with. gitdiff
produces the right output for a single commit (or gitformat-patch
for a series of commits).

Someone from the community will then be able to review the patch
and decide if it needs further work before integrating. If so,
they’ll leave comments on the trac ticket (trac will generally
email you if you’re marked as the owner, or you can explicitly
add yourself to the “cc” list for a ticket).

Github pull requests provide a web-based interface for review
and discussion of changes before they are accepted into
Xapian. Github’s documentation explains how you can go about
opening them.

If your patch is a sub-project in a larger piece of work, then
it’s important not to assume the patch is fine as it stands and to
immediately start the next sub-project. Instead you should
concentrate on completing the sub-project before moving on. Since
you’ll almost always have to wait at least a little time to get
feedback on any changes, you may want to put the code and tests up
while still working on documentation.

You should add further changes to pull requests by creating
additional commits locally, typically by using gitcommit--fixup,
and then pushing the branch up to Github. Only once everything’s
been approved should you squash your commits
together to keep the history clean.

Note

Once you’ve opened a pull request, you shouldn’t have to close
it until it’s merged (in which case we’ll generally close it for
you). Even if you need to redo some work, you can either add
fixup commits or (with agreement from whoever is reviewing the
PR) unwind your work and create completely new commits, force
pushing to replace the previous commits in the pull request.

It makes it much harder to review if you close a pull request in
the middle of a review only to open another with similar code.