[prev in list] [next in list] [prev in thread] [next in thread]
List: git
Subject: Re: [PATCH 2/5] t6300: refactor %(trailers) tests
From: Jeff King <peff () peff ! net>
Date: 2017-09-30 7:01:28
Message-ID: 20170930070127.xvtn7dfyuoh26mhp () sigill ! intra ! peff ! net
[Download RAW message or body]
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 <me@ttaylorr.com>
> ---
> t/t6300-for-each-ref.sh | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
The patch itself looks fine. :)
-Peff
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic