Re: [PATCH 2/3] fast-export: rename --signed-tags='warn' to 'warn-verbatim'

From: Luke Shumaker <lukeshu@lukeshu.com>
To: Taylor Blau <me@ttaylorr.com>
Cc: "Luke Shumaker" <lukeshu@lukeshu.com>,
	git@vger.kernel.org, "Luke Shumaker" <lukeshu@datawire.io>,
	"Junio C Hamano" <gitster@pobox.com>,
	"Elijah Newren" <newren@gmail.com>, "Jeff King" <peff@peff.net>,
	"Johannes Schindelin" <Johannes.Schindelin@gmx.de>,
	"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Subject: Re: [PATCH 2/3] fast-export: rename --signed-tags='warn' to 'warn-verbatim'
Date: Tue, 20 Apr 2021 09:45:30 -0600	[thread overview]
Message-ID: <87y2ddot7p.wl-lukeshu@lukeshu.com> (raw)
In-Reply-To: <YH4f97oreblENi3V@nand.local>

On Mon, 19 Apr 2021 18:27:35 -0600,
Taylor Blau wrote:
> 
> On Mon, Apr 19, 2021 at 04:54:40PM -0600, Luke Shumaker wrote:
> > From: Luke Shumaker <lukeshu@datawire.io>
> >
> > But still keep --signed-tags=warn as an undocumented alias.  This name
> > is clearer as it has symmetry with warn-strip:
> >
> >                    action              ->                   action
> >        +----------------------------+  ->       +----------------------------+
> >   msg? |      verbatim |      strip |  ->  msg? |      verbatim |      strip |
> >        |          warn | warn-strip |  ->       | warn-verbatim | warn-strip |
> >        +----------------------------+  ->       +----------------------------+
> 
> This table is rather confusing to me. What's unclear to me is what
> "msg?" and "action" are referring to. After reading your patch, I think
> it may be clearer to say:
> 
>     The --signed-tags option takes one of five arguments specifying how
>     to handle singed tags during export. Among these arguments, strip is
>     to warn-strip as verbatim is to warn. (The unmentioned argument is
>     'abort', which stops the fast-export process entirely). That is,
>     signatures are either stripped or copied verbatim while exporting,
>     with or without a warning.
> 
>     Make clear that the "warn" option instructs fast-export to copy
>     signatures verbatim by matching the pattern (and calling the option
>     "warn-verbatim").
> 
>     To maintain backwards compatibility, "warn" is still recognized as
>     an undocumented alias.

Thank you, I'll take much of this wording.

> > +test_expect_success 'signed-tags=warn' '
> > +	git fast-export --signed-tags=warn sign-your-name >output 2>err &&
> > +	grep PGP output &&
> > +	test -s err
> > +'
> > +
> > +test_expect_success 'signed-tags=warn-verbatim' '
> > +	git fast-export --signed-tags=warn sign-your-name >output 2>err &&
> 
> s/warn/warn-verbatim ?

Indeed, oops!

I'll also add a comment clarifying that the signed-tags=warn test is
testing for backward compatibility.

-- 
Happy hacking,
~ Luke Shumaker