Thanks for this work. Most things look fine with 1/2, comments on 2/2 below...
Victor Leschuk <[email protected]> wrote: > Add test for git-svn prefixed globs. Why a separate patch? Unless there's some documentation purpose for a regression, usually tests and a feature should be added atomically in the same commit. > --- /dev/null > +++ b/t/t9168-git-svn-prefixed-glob.sh > @@ -0,0 +1,136 @@ > +#!/bin/sh > +test_description='git svn globbing refspecs with prefixed globs' > +. ./lib-git-svn.sh > + > +cat > expect.end <<EOF We prefer redirects in new code to be in the form of ">foo" (no space) (or ">>foo" for append). It wasn't in the old tests, either, but Documentation/CodingGuidelines favors this for new code. > +the end > +hi > +start a new branch > +initial > +EOF All the setup code be checked for errors with '&&' as well. > + test "`git rev-parse refs/remotes/tags/t_end~1`" = \ > + "`git rev-parse refs/remotes/branches/b_start`" && > + test "`git rev-parse refs/remotes/branches/b_start~2`" = \ > + "`git rev-parse refs/remotes/trunk`" && And we prefer $(command) instead of `command` for nestability as Documentation/CodingGuidelines suggests. (yeah, most of the old tests don't follow the guidelines, but the guidelines also warn against fixup patches for them). Thanks again. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
