Re: [PATCH 2/5] t6300: refactor %(trailers) tests

[Jeff King]

On Fri, Sep 29, 2017 at 11:22:35PM -0700, Taylor Blau wrote:

> We currently have one test for %(trailers) in `git-for-each-ref(1)`, through
> "%(contents:trailers)". In preparation for more, let's add a few things:
> 
>   - Move the commit creation step to its own test so that it can be re-used.
> 
>   - Add a non-trailer to the commit's trailers to test that non-trailers 
> aren't
> shown using "%(trailers:only)".
> 
>   - Add a multi-line trailer to ensure that trailers are unfolded correctly
> using "%(trailers:unfold)".

This is a minor nit, but since you invited formatting critique in your
cover letter, I feel entitled. :)

Consider wrapping your commit messages (and emails in general) at
72 characters, rather than 80. That lets them show well on an 80-column
display even when indented by "git log" or by inline quoting in an email
reply.

I'm also of the opinion that while 80 characters is fine for code, it's
a bit wide for English text. You can find various claims online[1] from
people interested in typography that a line width of about 60-70
characters is pleasant for reading.

[1] E.g., https://baymard.com/blog/line-length-readability

> Signed-off-by: Taylor Blau 
> ---
>  t/t6300-for-each-ref.sh | 11 +--
>  1 file changed, 9 insertions(+), 2 deletions(-)

The patch itself looks fine. :)

-Peff

[PATCH 2/5] t6300: refactor %(trailers) tests

[Taylor Blau]

We currently have one test for %(trailers) in `git-for-each-ref(1)`, through
"%(contents:trailers)". In preparation for more, let's add a few things:

  - Move the commit creation step to its own test so that it can be re-used.

  - Add a non-trailer to the commit's trailers to test that non-trailers aren't
shown using "%(trailers:only)".

  - Add a multi-line trailer to ensure that trailers are unfolded correctly
using "%(trailers:unfold)".

Signed-off-by: Taylor Blau 
---
 t/t6300-for-each-ref.sh | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 834a9ed16..2a9fcf713 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -592,18 +592,25 @@ test_expect_success 'do not dereference NULL upon %(HEAD) 
on unborn branch' '
 cat >trailers <
 Signed-off-by: A U Thor 
+[ v2 updated patch description ]
+Acked-by: A U Thor
+  
 EOF
 
-test_expect_success 'basic atom: head contents:trailers' '
+
+test_expect_success 'set up trailers for next test' '
echo "Some contents" > two &&
git add two &&
-   git commit -F - <<-EOF &&
+   git commit -F - <<-EOF
trailers: this commit message has trailers
 
Some message contents
 
$(cat trailers)
EOF
+'
+
+test_expect_success 'basic atom: head contents:trailers' '
git for-each-ref --format="%(contents:trailers)" refs/heads/master 
>actual &&
sanitize_pgp actual.clean &&
# git for-each-ref ends with a blank line
-- 
2.14.1.145.gb3622a4ee