Re: [PATCHv2 1/5] t7004 (tag): modernize style

Am 03.03.2012 03:15, schrieb Tom Grennan:

-test_expect_success 'trying to delete an already deleted tag should fail' \
-	'test_must_fail git tag -d mytag'
+test_expect_success \
+	'trying to delete an already deleted tag should fail' '
+	test_must_fail git tag -d mytag
+'

Personally, I prefer lines with 80+ chars over lines broken with a
backslash. So, while I can understand that you do not always remove a
backslash to avoid churn, I think it is not warranted to *introduce* one
to shorten a line below 80 chars. But that's really just a matter of taste.

 test_expect_success GPG \
 	'creating a signed tag with -F messagefile should succeed' '
+	cat >sigmsgfile <<-EOF
+		Another signed tag
+		message in a file.
+	EOF
+	get_tag_header file-signed-tag $commit commit $time >expect <sigmsgfile
+	echo "-----BEGIN PGP SIGNATURE-----" >>expect

Several && missing here and later in the file.

I stopped reading here. This is too much to read in one go. This should
really be broken into several parts. For example, you changed
get_tag_header and added several <emptyfile redirections (which should
be /dev/null IMO); that has not a lot to do with "modernize style".

 	git tag -s -F sigmsgfile file-signed-tag &&
 	get_tag_msg file-signed-tag >actual &&
 	test_cmp expect actual
 '

-- Hannes