Re: [PATCH 2/2] git-svn: test for git-svn prefixed globs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Thanks for this work.  Most things look fine with 1/2, comments
on 2/2 below...

Victor Leschuk <vleschuk@xxxxxxxxx> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]