Re: [PATCH v4 2/2] http-fetch: redact url on die() message

From: Ivan Frade <ifrade@google.com>
Date: Oct 28, 2021, 22:44 UTC
Message-ID: <CANQMx9VAgj8hLkd2+hqRHdD2+zNQ8T0jUTqpzmJE65Z2UtTnDQ@mail.gmail.com>
In-Reply-To: <CAPig+cTKSp28oUvESCWLB+OLBjbUSt3vhz6n3eVmkfYf9arcrg@mail.gmail.com>

On Thu, Oct 28, 2021 at 10:26 AM Eric Sunshine <sunshine@sunshineco.com> wrote:

>
> On Thu, Oct 28, 2021 at 12:46 PM Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:

Show 16 quoted lines

>
> > nit: die() messages should start with lower-case (in CodingGuidelines), and I think it's better to quote both, so:
> >
> >     die("unable to get pack '%s': '%s'", ...)
> >
> > Or maybe without the second '%s', as in 3e8084f1884 (http: check
> > CURLE_SSL_PINNEDPUBKEYNOTMATCH when emitting errors, 2021-09-24) (which
> > I authored, but just copy/pasted the convention in the surrounding
> > code)>
>
> Note that this is not a new die() call; it just got indented as-is by
> this patch, so the changes you suggest to the message string are
> potentially outside the scope of this patch. Possibilities: (1) make
> the changes in this patch but mention them in the commit message; (2)
> make the changes in a preparatory patch; (3) punt on the changes for
> now.

I went for option (1). It is a minimal change and we are moving that line in this commit.

Ivan