[darcs-users] darcs patch: Test for Issue 1473 (darcs annotate misbehaves)

[Trent W. Buck]

Looks OK to me, though I'd appreciate a follow-up or amended patch to
address the nitpicking below.

Florian Gilcher <flo at andersground.net> writes:

> Fri May 29 18:44:09 CEST 2009  Florian Gilcher <flo at andersground.net>
>   * Test for Issue 1473 (darcs annotate misbehaves)

Though currently no code handles it, I have adopted the convention of
naming such patches "Accept issueNNNN: foo won't frobozz."  This mimics
the closing convention of "Resolve issueNNNN: make foo frobozz."

>   This is a test for issue 1473, "darcs annotate --repodir /something '.' fails in darcs 2.2".
>
>   Original Message for reference: [...]

This is a relatively long-winded patch description.  I would prefer the
patch description to either be paraphrased down to the important parts,
or simply a URL to the original message in the osuosl pipermail archive.

> ] addfile ./bugs/issue1473_annotate_repodir.sh
> hunk ./bugs/issue1473_annotate_repodir.sh 1
> +#!/usr/bin/env bash
> +set -ev

There should be a comment here briefly describing what is being tested,
who holds copyright on the work and (most importantly) the license
declaration.  You can find an example in tests/EXAMPLE.sh.

> +mkdir temp
> +cd temp
> +darcs init
> [...]
> +darcs add --rec .

I personally prefer to skip the mkdir with init --repo, and skip the
darcs add with record -l, but I don't care enough to push for it.

> +darcs record -a -m ab -A test

Setting the author shouldn't be necessary when using the test framework,
though it shouldn't matter.

> +darcs annotate a/a
> +# annotate --repodir=something '.' should work
> +cd ..
> +darcs annotate --repodir temp '.'

> +cd temp
> +cd ..

Aren't these cd's a noop?

(As you can see in my tests, I actually don't like using cd at all,
since it changes implicit state, making it harder to copy-and-paste a
subset of a test into a shell.  Yay for --repodir.)