On Sat, May 30, 2009 at 12:35:55 +1000, Trent W. Buck wrote: > Looks OK to me, though I'd appreciate a follow-up or amended patch to > address the nitpicking below.
I've applied the patch, but Florian, if you care to pick nits with us, I've highlighted three points that a small patch could address. > 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." Moot, now that I've applied the patch. But I'll trust Trent if he thinks it's a good habit for us to get into (with the added automation that he suggests) > 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. Also moot now. > > 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. 1. This should be fairly easy to address, if you wouldn't mind. > 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. 2. Might as well, but it doesn'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? 3. Another might as well > (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.) And in case you missed the Jason/Trent follow-up, please disregard this remark for this particular test :-) -- Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow> PGP Key ID: 08AC04F9
signature.asc
Description: Digital signature
_______________________________________________ darcs-users mailing list [email protected] http://lists.osuosl.org/mailman/listinfo/darcs-users
