Looks OK to me, though I'd appreciate a follow-up or amended patch to address the nitpicking below.
Florian Gilcher <[email protected]> writes: > Fri May 29 18:44:09 CEST 2009 Florian Gilcher <[email protected]> > * 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.) _______________________________________________ darcs-users mailing list [email protected] http://lists.osuosl.org/mailman/listinfo/darcs-users
