May, 2012

15
May 12

$HOME/.ssh/ directory permissions

Because I always forget what permissions SSH needs to make public key authentication work:

chmod go-w $HOME/
chmod 700 $HOME/.ssh/
chmod 600 $HOME/.ssh/*

4
May 12

Our Git development workflow

Over the last couple of years we’ve been tweaking our development workflow and it has been really successful for us. It certainly isn’t revolutionary (it is heavily based on the Linux kernel), but it has worked so well and our code quality has improved so considerably that I feel it’s worth writing about. We are a very small team – typically 2 or 3 developers per-project – but I imagine this workflow will only get better at scale. A happy side-effect is that all of our projects are completely setup for remote working. I’m really loving this workflow.

Each project has a maintainer, who is also a developer. This role is simply to be the gatekeeper of origin/master of the Git repo. The maintainer is responsible for checking that a patch has the appropriate tags and is ready for inclusion (see the requirements in point 6 below). These concrete rules (which we do break, occasionally) force us to code review. Contrary to my expectations, I really enjoy the constant review process, from both sides.

We create a new local branch for every feature or bugfix. The workflow of a project is focussed around a PROJECT-devel mailling list. This is where all the patches are posted, reviewed and commented on before being pushed. The reviews are without doubt the biggest win here. We have adopted a strict “no change too small” policy – even a misplaced comma in a comment should be picked up on. This can feel pretty harsh at first, but I’ve found that we soon stop being precious and end up writing much better code anyway. Also, because we’re absolutely sure our commit messages will be read before even touching the repository, they have become a lot more detailed and actually useful.

For the nitty-gritty details, here’s the commit documentation from our projects in full:

doc/commit_workflow:

Commit workflow
===

This project is run in a similar vein to the Linux kernel. You will need to
subscribe to <PROJECT-devel@lists.example.com>.

The project is currently maintained by Mark Somerville <mark@example.com>.

The origin/master branch should _always_ be in a deployable state (and almost
certainly be what is actually deployed).


Some notes
===

* Most code changes should come with appropriate accompanying tests. This
  obviously doesn't apply to things like typo's and there are some other things
  where testing doesn't make sense.


Tags
===

Patches should come appropriately tagged. There are various tags that _should_
be used and others which aren't always required/appropriate.

* Signed-off-by: is the main one. There should be at least one of these on all
  patches by at least the author. Anyone else who handled or was involved in
  the patch or was in its delivery path should also add their Signed-off-by:
  e.g the committer.

  Signing-off a patch is an explicit acknowledgement that you know where it
  came from and that it is, to the best of your knowledge, eligible for
  inclusion in the project.

  The tag would look like:

  Signed-off-by: Mark Somerville <mark@example.com>

* Reviewed-by: says that the reviewer has performed a code review and that the
  changes make sense, look sane and are generally appropriate for inclusion.

  The tag would look like:

  Reviewed-by: Mark Somerville <mark@example.com>

* Tested-by: says that the tester has applied the patch and at least run all of
  the tests using whatever testing makes sense for the given context - it could
  be running `rake` or looking at it in a browser. More thorough tests can be
  performed, of course (and probably should be in some cases). The tag should
  relevant software and versions. For example, it will often include the Ruby
  interpreter and version used.

  We want to support at least MRI (Matz's Ruby Interpreter, the "standard"
  Ruby), version 1.8.7. Some interpreters are below:

  * MRI 1.9.3
  * MRI 1.8.7
  * RBX 1.8.7
  * JRuby 1.9.2

  The tag would look something like:

  Tested-by: Mark Somerville <mark@example.com> [MRI 1.9.3, RBX 1.8.7]
  Tested-by: Mark Somerville <mark@example.com> [RHEL5, MRI 1.9.3, Rails 3.0.11]

* Co-authored-by: is applied to patches that were written by more than one
  author. One example is pair-programming, where the code was created on one
  workstation. In this case the Git Author metadata is set to the owner of the
  workstation and this author would add a Signed-of-by tag, as usual. The other
  developer would add both a Co-authored-by and a Signed-off-by tag.

  The tag would look like:

  Co-authored-by: Mark Somerville <mark@example.com>


Tag ordering
===

Developers can and should add multiple tags to patches. The basic rule is that
a single developer's Signed-off-by tag should come after his others. If other
developers add tags to the patch, they should be added below.

Example: Mark was the author and Euan was the maintainer at the time:

  Tested-by: Mark Somerville <mark@example.com> [MRI 1.9.3]
  Signed-off-by: Mark Somerville <mark@example.com>
  Tested-by: Mike Lauder <mike@example.com> [MRI 1.8.7]
  Reviewed-by: Julia Allison <julia@example.com>
  Signed-off-by: Euan Maxwell <euan@example.com>

Example: Mark and Julia co-authored the patch on Mark's workstation and Euan was
the maintainer at the time:

  Tested-by: Mark Somerville <mark@example.com> [MRI 1.9.3]
  Signed-off-by: Mark Somerville <mark@example.com>
  Co-authored-by: Julia Allison <julia@example.com>
  Signed-off-by: Julia Allison <julia@example.com>
  Tested-by: Mike Lauder <mike@example.com> [MRI 1.8.7]
  Signed-off-by: Euan Maxwell <euan@example.com>

An exception to the Signed-off-by tag being at the end of a developer's tags
but above some others is when the author is also the project maintainer. In
this case, the author's Signed-off-by is the final tag, because he was the last
person to touch the patch (by committing it).

Example: Mark authored the patch and was also the maintainer at the time:

  Tested-by: Mark Somerville <mark@example.com> [MRI 1.9.3]
  Reviewed-by: Julia Allison <julia@example.com>
  Tested-by: Mike Lauder <mike@example.com> [MRI 1.8.7]
  Signed-off-by: Mark Somerville <mark@example.com>


Tag comments
===

On occasion, a comment should be added to a tag. Probably the most common use
of this is when the maintainer is ready to push the patch, but wants to perform
some minor tidy-up first (whitespace fixes, commit message alterations).
Because the change is minor this is easier than re-posting to the list.

The comments should be above the tag that is commented on and be of the form:

  [ mark@example.com: just a wee comment, probably no more than a line ]

An example:

  Tested-by: Julia Allison <julia@example.com> [MRI 1.9.3]
  Signed-off-by: Julia Allison <julia@example.com>
  Tested-by: Mark Somerville <mark@example.com> [MRI 1.9.3]
  [ mark@example.com: whitespace and refactored to idomatic Rails ]
  Signed-off-by: Mark Somerville <mark@example.com>


Merging a change
===

1. When you have made a change that you are happy with, you should commit it to
   your local Git repo with a Signed-off-by tag (see notes above). For commits
   involving code, you probably want to attach a Tested-by tag too.

   This is a good note about commit messages:

   http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html

2. Generate a patch, probably using `git-format-patch` (or `git-send-email`
   directly).

3. Email the patch to the PROJECT-devel mailing list (git-send-email is good
   for this) with the subject: "[PATCH] First line of commit"
   (`git-format-patch` will do this for you).

4. When your feature/fix spans multiple commits, you should probably email a
   patchset. This consists of a summary email with a subject like:

     "[PATCH 0/N] Add a teleporting time machine"

   with the commit patches as separate emails in reply to the summary.
   `git-send-email --compose` is very useful for this.

5. Developers comment on the patch(set). Sometimes this will be as simple as
   adding some tags to the commit. Multiple tags by different developers are
   fine (and welcomed).

   Other times, some changes will be needed before the team agree the patch is
   acceptable. If this is the case, you should incorporate the changes and go
   back to stage 2. The difference this time around is that when posting the
   patch during stage 3, the start of the subject should be changed to
   [PATCH v2] to indicate that this is the new version of the patch to review
   and test.

   You should also update the patch changelog. This versioning belongs below
   the commit message and above the diff stat (just below the ---). An example:

     Wrote pseudo-code algorithm for predicting lottery numbers

     Signed-off-by: Mark Somerville <mark@example.com>
     Co-authored-by: Euan Maxwell <euan@example.com>
     Signed-off-by: Euan Maxwell <euan@example.com>
     ---
      v2 - Added better comments clarifying the algorithm

      doc/lottery_number_prediction | 154 +++++++++++++++++++++++---------------
      1 files changed, 125 insertions(+), 29 deletions(-)

   When sending this v2 patch, be sure to email the patch as a reply to the
   appropriate patch thread, using the Message-ID you want to reply to.

6. To be acceptable for pushing to master, a commit must have at least one
   Signed-off-by tag (the author) and one or more Reviewed-by/Co-authored-by
   tags. There must also be at least one Tested-by tag that is by someone other
   than the original author (this is to protect against things only working due
   to a quirk of a single development or testing machine). Tested-by tags can
   be omitted for commits that don't need any testing or where it doesn't make
   sense, like documentation changes.

7. Once the patch is acceptable, it is the maintainers responsibility to apply,
   amend the commit to add the extra tags and push it to origin.