Subject: Re: [PATCH v5 09/15] bugreport: generate config safelist based on docs

From: Emily Shaffer <[email-masked]>
Date: Tue, 4 Feb 2020 16:30:43 -0800

On Fri, Jan 31, 2020 at 10:20:05PM +0100, Martin Ågren wrote:
> On Fri, 24 Jan 2020 at 04:35, <[email-masked]> wrote:
> > Add a new step to the build to generate a safelist of git-config
> > variables which are appropriate to include in the output of
> > git-bugreport. New variables can be added to the safelist by annotating
> > their documentation in Documentation/config with the
> > "bugreport" macro, which is recognized by AsciiDoc and
> > AsciiDoctor.
> 
> "which is recognized by" sounds like it's built-in. Maybe "with a new
> no-op 'bugreport' macro" or something like that.

Sure, I'll wordsmith this a little.

> 
> > -sendemail.signedoffcc (deprecated)::
> > +sendemail.signedoffcc (deprecated) bugreport:exclude[x] ::
> >         Deprecated alias for `sendemail.signedoffbycc`.
> >
> > -sendemail.smtpBatchSize::
> > +sendemail.smtpBatchSize bugreport:include[x] ::
> >         Number of messages to be sent per connection, after that a relogin
> >         will happen.  If the value is 0 or undefined, send all messages in
> >         one connection.
> >         See also the `--batch-size` option of linkgit:git-send-email[1].
> 
> Do we really need to list includes *and* excludes?

Need? Nah. The exclude markers are not even acknowledged by the current
implementation.

> I could see the point of adding an exclude here and there to signal
> that "this might look innocent enough, but trust me, we really need to
> exclude this" in order to avoid future commits to more or less
> accidentally over-include.

This was more of my line of thinking, along with the thought that some
folks may use bug reporting internally, where they have A) more control
over which extensions/tools are in use, and B) less concern about
sharing things like remote URLs, branch names, etc. and may want to go
for a blocklist as opposed to a safelist, to make it easier for their
support folks to diagnose.

> Should we add some internal documentation and/or a remark in the commit
> message about this? As an example, is "sendemail.signedofcc" sensitive
> enough that we need to explicitly exclude it? If someone wants to come
> along and include it, they don't just need to argue for an inclusion,
> but also for lifting the exclusion. Hmm?

Yeah, I think this is a good point. I'll try to figure out how to reword
the commit message, and take another look at the sample include/exclude
change I made to see where I can omit entirely.

 - Emily