Re: [PATCH] git-svn: Support for git-svn propset

From: Torsten Bögershausen <tboegi@web.de>
Date: Dec 7, 2014, 05:45 UTC
Message-ID: <5483E968.6060708@web.de>
In-Reply-To: <20141206222942.GB91825@elvis.mu.org>

Show 17 quoted lines

> diff --git a/t/t9148-git-svn-propset.sh b/t/t9148-git-svn-propset.sh
> new file mode 100755
> index 0000000..b36a8a2
> --- /dev/null
> +++ b/t/t9148-git-svn-propset.sh
> @@ -0,0 +1,71 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2014 Alfred Perlstein
> +#
> +
> +test_description='git svn propset tests'
> +
> +. ./lib-git-svn.sh
> +
> +foo_subdir2="subdir/subdir2/foo_subdir2"
> +

In case something goes wrong (for whatever reason): do we need a && chain here ?

Show 5 quoted lines

> +mkdir import
> +(cd import
> +	mkdir subdir
> +	mkdir subdir/subdir2
> +	touch foo		# for 'add props top level'

"touch foo" can be written shorter:

>foo

Show 6 quoted lines

> +	touch subdir/foo_subdir # for 'add props relative'
> +	touch "$foo_subdir2"	# for 'add props subdir'
> +	svn_cmd import -m 'import for git svn' . "$svnrepo" >/dev/null
> +)
> +rm -rf import
> +

> +test_expect_success 'initialize git svn' 'git svn init "$svnrepo"'
> +test_expect_success 'fetch revisions from svn' 'git svn fetch'

This may look a little bit strange, 2 times test_expect_success in a row, is the indentention OK ?

> +
> +# There is a bogus feature about svn propset which means that it will only
> +# be taken as a delta for svn dcommit iff the file is also modified.
> +# That is fine for now.

"there is a bogus feature ?" Small typo: s/iff/if/ How about this: #The current implementation has a restriction: #svn propset will be taken as a delta for svn dcommit only #if the file content is also modified

Show 9 quoted lines

> +test_expect_success 'add props top level' '
> +	git svn propset svn:keywords "FreeBSD=%H" foo &&
> +	echo hello >> foo &&
> +	git commit -m "testing propset" foo &&
> +	git svn dcommit
> +	svn_cmd co "$svnrepo" svn_project &&
> +	(cd svn_project && test "`svn propget svn:keywords foo`" = "FreeBSD=%H") &&
> +	rm -rf svn_project
> +	'

Is there a reason why there is no "&&" after "git svn dcommit" ? If yes, it could be better to make this really clear to the readers and write (This idea is stolen from Peff)

{ git svn dcommit || true } &&

Show 14 quoted lines

> +
> +test_expect_success 'add multiple props' '
> +	git svn propset svn:keywords "FreeBSD=%H" foo &&
> +	git svn propset fbsd:nokeywords yes foo &&
> +	echo hello >> foo &&
> +	git commit -m "testing propset" foo &&
> +	git svn dcommit
> +	svn_cmd co "$svnrepo" svn_project &&
> +	(cd svn_project && test "`svn propget svn:keywords foo`" = "FreeBSD=%H") &&
> +	(cd svn_project && test "`svn propget fbsd:nokeywords foo`" = "yes") &&
> +	(cd svn_project && test "`svn proplist -q foo | wc -l`" -eq 2) &&
> +	rm -rf svn_project
> +	'
> +

Ah, another small thing: the "wc -l" will not work under Mac OS X. Please see test_line_count() in t/test-lib-functions.sh

And thanks for improving Git