Peer review for GSoC pull requests

[James Aylett]

Hi everyone,

To our GSoC students, I hope you are learning a ton this summer. We
are enjoying working with you all.

We've been discussing how we can improve the GSoC experience for our
students, and one of the things that has come up is the importance in
open source projects of contributors reviewing each others'
changes. Therefore we're inviting all our students in particular to
get involved in reviewing pull requests (whether from GSoC students or
from others).

There are various benefits for people new to Xapian or to open source
collaboration:

1. Gives a chance to interact with other GSoC students

2. Gives you insight into other parts of Xapian that you aren't
   working on

3. Reviews are a good way to become a better programmer; you can see
   how someone else is tackling a problem, and compare it to how you
   might approach it. One or both can learn from the differences!

4. In larger open source projects, it's essential to have more people
   available to review pull requests, so that work on the project
   isn't dependent on the time of just one or two people. As Xapian
   grows in contributors, this is increasingly important to us.

Some quick tips on review pull requests:

1. If something isn't clear, ask! It may be that you don't know a part
   of Xapian, but it may equally be that the code is difficult to follow
   and would benefit from some comments or documentation explaining it.

2. Every change should have documentation and tests, where that makes
   sense.

3. A good pull request is made up of good commits. I've written
   something about good commits in the Xapian developer guide [1].
   Generally speaking, most commits should be fairly small -- at most
   a few hundred lines of changes (and often much smaller). If a PR is
   one giant commit, that's usually a sign something isn't quite right.

4. Code reviews should be respectful, with people helping each
   other. Thoughtbot has a good guide for reviewing code [2].

5. Don't spend too long in one go on code review; it can be tiring. A
   maximum of 20-30 minutes is a good idea when you're starting out,
   although some pull requests won't take that long.

You can send an email to the xapian-devel mailing list [3] asking
other students and members of the community to review your newly
raised pull request. If you're interested, you can also 'watch' the
Xapian project on github [4], which will notify you when new pull
requests are opened.

Generally, discussions can remain on the pull request, but if
something comes up that no one involved in the review knows how to
deal with, it's best to bring it to the mailing list or the #xapian
channel on IRC.

Right now, the following pull requests are up for review:

 * PL2+ weighting scheme: https://github.com/xapian/xapian/pull/108

 * Instructions / changes for cross-compiling for Android:
   https://github.com/xapian/xapian/pull/103 -- this was originally
   proposed as a GSoC project, but unfortunately we weren't able to
   accept it, so it's great to see it being worked on anyway!

 * Test suite for xapian-letor:
   https://github.com/xapian/xapian/pull/110

That last one is a bit large and might seem overwhelming at first, but
may be interesting for people who haven't looked at the learning to
rank ("letor") framework previously. Similarly, there's a PR that's
about ready to be merged that adds BM25+ support to our collection of
weighting schemes (https://github.com/xapian/xapian/pull/104), which
gives an idea of how to work in that part of the system.

(There are others, but in many cases the contributor hasn't had time
to address earlier comments. The older ones were also before we
configured Travis to build pull requests automatically -- which you
can see as little green ticks in PR and commit lists. Over time I'm
hoping to go back and clear up old PRs.)


J


[1] https://xapian-developer-guide.readthedocs.io/en/latest/contributing/workflow.html#make-commits-out-of-your-changes

[2] https://github.com/thoughtbot/guides/tree/master/code-review

[3] xapian-devel at lists.xapian.org /
    https://lists.xapian.org/mailman/listinfo/xapian-devel

[4] https://github.com/xapian/xapian

-- 
  James Aylett, occasional trouble-maker
  xapian.org