From: Jeff King <peff@peff.net>
Date: Wed, 17 Oct 2018 18:11:56 GMT
Subject: Re: [RFC] revision: Add --sticky-default option
Message-ID: <20181017181156.GA28326@sigill.intra.peff.net>
In-Reply-To: <alpine.DEB.2.21.1810170620260.166761@matvore-linux.svl.corp.google.com>

On Wed, Oct 17, 2018 at 06:24:05AM -0700, Matthew DeVore wrote:

> > Yuck, t4202 is a mix of older and newer styles. I'm OK with this as-is
> > because you've matched the surrounding code, but these days I'd probably
> > write:
> > 
> >  test_expect_success '--sticky-default ^<rev>' '
> > 	{
> > 		echo sixth
> > 		echo fifth
> > 	} >expect &&
> > 	git log --format=%s --sticky-default ^HEAD~2 >actual &&
> > 	test_cmp expect actual
> >  '
> > 
> 
> How about test_write_lines? That is a little more readable to me than
> the echos in a subshell. A patch was recently queued with a usage of
> that function:

Ah, yeah, that would be fine. I was trying to avoid a cat/here-doc combo
since it incurs a process, but test_write_lines is readable and fast.

The main style things I wanted to show are:

  - setting up the expect file should go in the test_expect block

  - no space between ">" and the filename

Those are both present in the surrounding code, but we're slowly
cleaning them up.

-Peff