Re: [PATCH 2/1] SubmittingPatches: not git-secur...@googlegroups.com

2018-05-29 Thread brian m. carlson
On Tue, May 29, 2018 at 07:02:03PM +0100, Thomas Gummerer wrote:
> On 05/28, Junio C Hamano wrote:
> > This is a tangent, but the use of footnote below looks a but
> > curious.  How would {1} reference pick which :1: to use?  The
> > closest preceding one?
> 
> Tbh I didn't look at the docs for doing this, but just used the same
> syntax as we're already using and tried it with both asciidoc and
> asciidoctor.  And yes it seems like it always picks the preceeding
> one.

Yes, I believe the attributes namespace is flat and substituted using
the current version that's defined.  I wouldn't rely extensively on
that, though, so unique names are probably better.

> > As this appears on a page that already has other footnotes attached
> > to an adjacent paragraph, I am wondering if they should be made into
> > a part of the same numbering sequence.
> 
> I have now actually looked at the docs, and this numbering has nothing
> to do with the footnote format, but rather is used to substitute the
> attribute that's specified in the curly braces with the text that's
> after :: [1].  This initially confused me a bit.  Maybe it
> would be nicer to give the attributes names instead of just numbers?
> As we keep adding footnotes, that would be less likely to produce
> conflicts between the different attributes I think.
> I'm also adding brian to the cc list, as he first converted this to
> AsciiDoc for opinions.

In AsciiDoc, footnotes use the named macro syntax.  I thought it would
be difficult to read to have the footnotes inline, so I chose to use an
attribute to substitute them.  I used numbers because we had a small
number of them and the original footnotes were numbered.  I was trying
to make a minimal, faithful conversion.

I have no objection to named footnotes and I agree they're easier to use
if we have a large number of them.  I think whatever we use, we should
try to make them unique, as I mentioned above.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature


Re: [PATCH 2/1] SubmittingPatches: not git-secur...@googlegroups.com

2018-05-29 Thread Thomas Gummerer
On 05/29, Thomas Gummerer wrote:
> On 05/28, Junio C Hamano wrote:
> > Thomas Gummerer  writes:
> > 
> > > Yeah sorry, that's what I meant.
> > > https://public-inbox.org/git/20180308150820.22588-1-ava...@gmail.com/
> > > is the reference I meant to put there.
> > >
> > > How about something like the below?  This is tested with asciidoc
> > > 8.6.10 and asciidoctor 1.5.6.2.  I'm also happy to squash the two
> > > patches into one if that's preferred.
> > >
> > 
> > If the discussion in the proposed log message needs to be updated
> > anyway, it is a good opportunity to make them into a single patch,
> > as they share exactly the same objective.
> 
> This was mostly a clarification of the note I added after the '---',
> but I'm happy to just make this one patch either way.
> 
> > This is a tangent, but the use of footnote below looks a but
> > curious.  How would {1} reference pick which :1: to use?  The
> > closest preceding one?
> 
> Tbh I didn't look at the docs for doing this, but just used the same
> syntax as we're already using and tried it with both asciidoc and
> asciidoctor.  And yes it seems like it always picks the preceeding
> one.
> 
> > As this appears on a page that already has other footnotes attached
> > to an adjacent paragraph, I am wondering if they should be made into
> > a part of the same numbering sequence.
> 
> I have now actually looked at the docs, and this numbering has nothing
> to do with the footnote format, but rather is used to substitute the
> attribute that's specified in the curly braces with the text that's
> after :: [1].  This initially confused me a bit.  Maybe it
> would be nicer to give the attributes names instead of just numbers?
> As we keep adding footnotes, that would be less likely to produce
> conflicts between the different attributes I think.
> 
> I'm also adding brian to the cc list, as he first converted this to
> AsciiDoc for opinions.

Now really adding the CC, I failed earlier.  Sorry about the noise.

> [1]: 
> https://asciidoctor.org/docs/asciidoc-syntax-quick-reference/#attributes-and-substitutions


Re: [PATCH 2/1] SubmittingPatches: not git-secur...@googlegroups.com

2018-05-29 Thread Thomas Gummerer
On 05/28, Junio C Hamano wrote:
> Thomas Gummerer  writes:
> 
> > Yeah sorry, that's what I meant.
> > https://public-inbox.org/git/20180308150820.22588-1-ava...@gmail.com/
> > is the reference I meant to put there.
> >
> > How about something like the below?  This is tested with asciidoc
> > 8.6.10 and asciidoctor 1.5.6.2.  I'm also happy to squash the two
> > patches into one if that's preferred.
> >
> 
> If the discussion in the proposed log message needs to be updated
> anyway, it is a good opportunity to make them into a single patch,
> as they share exactly the same objective.

This was mostly a clarification of the note I added after the '---',
but I'm happy to just make this one patch either way.

> This is a tangent, but the use of footnote below looks a but
> curious.  How would {1} reference pick which :1: to use?  The
> closest preceding one?

Tbh I didn't look at the docs for doing this, but just used the same
syntax as we're already using and tried it with both asciidoc and
asciidoctor.  And yes it seems like it always picks the preceeding
one.

> As this appears on a page that already has other footnotes attached
> to an adjacent paragraph, I am wondering if they should be made into
> a part of the same numbering sequence.

I have now actually looked at the docs, and this numbering has nothing
to do with the footnote format, but rather is used to substitute the
attribute that's specified in the curly braces with the text that's
after :: [1].  This initially confused me a bit.  Maybe it
would be nicer to give the attributes names instead of just numbers?
As we keep adding footnotes, that would be less likely to produce
conflicts between the different attributes I think.

I'm also adding brian to the cc list, as he first converted this to
AsciiDoc for opinions.

[1]: 
https://asciidoctor.org/docs/asciidoc-syntax-quick-reference/#attributes-and-substitutions

> > @@ -264,6 +264,11 @@ people who are involved in the area you are touching 
> > (the `git
> >  contacts` command in `contrib/contacts/` can help to
> >  identify them), to solicit comments and reviews.
> >  
> > +:1: footnote:[The Git Security mailing list: git-secur...@googlegroups.com]
> > +
> > +Patches which are security relevant should be submitted privately to
> > +the Git Security mailing list{1}.
> > +
> >  :1: footnote:[The current maintainer: gits...@pobox.com]
> >  :2: footnote:[The mailing list: git@vger.kernel.org]
> 
> Also, the placement of this new paragraph is rather odd.  
> 
> I am guessing that the reason why you put it _before_ the normal
> list address is to make sure those with secrets that must be guarded
> won't send it to the list first without thinking, but then this
> place is too late for that, as the previous paragraph already told
> the reader that the patch should be sent to the list and others but
> not necessarily to the maintainer.  This should go one paragraph
> before that, at least.  I briefly considered suggesting to move it
> even earlier, e.g. the beginning of "Sending your patches" section,
> but then by the time readers with potential security patches may
> have forgotten it, or worse, get confused by us, when we say "Send
> your patches with To: set to the list".  So I dunno.  The most
> conservative would be to write it at the beginning of the section
> and then repeat it just before "Send to the list, Cc releavant
> people" paragraph as a reminder.

Yeah I wasn't quite sure where to best fit this in.  I'd be happy with
it appearing twice.  Will update this in v2.


Re: [PATCH 2/1] SubmittingPatches: not git-secur...@googlegroups.com

2018-05-27 Thread Junio C Hamano
Thomas Gummerer  writes:

> Yeah sorry, that's what I meant.
> https://public-inbox.org/git/20180308150820.22588-1-ava...@gmail.com/
> is the reference I meant to put there.
>
> How about something like the below?  This is tested with asciidoc
> 8.6.10 and asciidoctor 1.5.6.2.  I'm also happy to squash the two
> patches into one if that's preferred.
>

If the discussion in the proposed log message needs to be updated
anyway, it is a good opportunity to make them into a single patch,
as they share exactly the same objective.

This is a tangent, but the use of footnote below looks a but
curious.  How would {1} reference pick which :1: to use?  The
closest preceding one?  

As this appears on a page that already has other footnotes attached
to an adjacent paragraph, I am wondering if they should be made into
a part of the same numbering sequence.

> @@ -264,6 +264,11 @@ people who are involved in the area you are touching 
> (the `git
>  contacts` command in `contrib/contacts/` can help to
>  identify them), to solicit comments and reviews.
>  
> +:1: footnote:[The Git Security mailing list: git-secur...@googlegroups.com]
> +
> +Patches which are security relevant should be submitted privately to
> +the Git Security mailing list{1}.
> +
>  :1: footnote:[The current maintainer: gits...@pobox.com]
>  :2: footnote:[The mailing list: git@vger.kernel.org]

Also, the placement of this new paragraph is rather odd.  

I am guessing that the reason why you put it _before_ the normal
list address is to make sure those with secrets that must be guarded
won't send it to the list first without thinking, but then this
place is too late for that, as the previous paragraph already told
the reader that the patch should be sent to the list and others but
not necessarily to the maintainer.  This should go one paragraph
before that, at least.  I briefly considered suggesting to move it
even earlier, e.g. the beginning of "Sending your patches" section,
but then by the time readers with potential security patches may
have forgotten it, or worse, get confused by us, when we say "Send
your patches with To: set to the list".  So I dunno.  The most
conservative would be to write it at the beginning of the section
and then repeat it just before "Send to the list, Cc releavant
people" paragraph as a reminder.


[PATCH 2/1] SubmittingPatches: not git-secur...@googlegroups.com

2018-05-27 Thread Thomas Gummerer
On 05/27, Jonathan Nieder wrote:
> Thomas Gummerer wrote:
> 
> > Add a mention of the security mailing list to the README.
> > 2caa7b8d27 ("git manpage: note git-secur...@googlegroups.com",
> > 2018-03-08) already added it to the man page, but I suspect that for
> > many developers, such as myself, the README would be the first place
> > to go looking for it.
> >
> > Use the same wording as we already have on the git-scm.com website and
> > in the man page.
> >
> > Signed-off-by: Thomas Gummerer 
> > ---
> >  README.md | 3 +++
> >  1 file changed, 3 insertions(+)
> 
> Reviewed-by: Jonathan Nieder 

Thanks!

> > 2caa7b8d27 ("git manpage: note git-secur...@googlegroups.com",
> > 2018-03-08) also mentions SubmittingPatches, but I think people are
> > much more likely to submit a report of a security issue first, rather
> > than sending a patch, for which I think the README is more useful.
> 
> I don't see a mention of SubmittingPatches in "git show 2caa7b8d27"
> output.  git help git tells me:
> 
>   Report bugs to the Git mailing list 
>   where the development and maintenance is primarily done. You
>   do not have to be subscribed to the list to send a message
>   there.
> 
>   Issues which are security relevant should be disclosed
>   privately to the Git Security mailing list
>   .
> 
> Do you mean that the discussion around that change suggested updating
> SubmittingPatches too?  The "Sending your patches" section indeed
> mentions git@vger.kernel.org, so a mention of the security list would
> indeed be welcome there, even though typically the discussion has
> already started there before a patch is written.

Yeah sorry, that's what I meant.
https://public-inbox.org/git/20180308150820.22588-1-ava...@gmail.com/
is the reference I meant to put there.

How about something like the below?  This is tested with asciidoc
8.6.10 and asciidoctor 1.5.6.2.  I'm also happy to squash the two
patches into one if that's preferred.

--->8---

The previous commit added a note about the Git Security mailing list
to the README.  Add it to Documentation/SubmittingPatches as well, so
developers trying to submit a security relevant patch are pointed in
the right direction.

The wording is adjusted slightly compared to the git-scm.com website
and the README, as they are talking about issues, while
SubmittingPatches is talking about patches.

Signed-off-by: Thomas Gummerer 
---
 Documentation/SubmittingPatches | 5 +
 1 file changed, 5 insertions(+)

diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index 945f8edb46..aeb7948d98 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -264,6 +264,11 @@ people who are involved in the area you are touching (the 
`git
 contacts` command in `contrib/contacts/` can help to
 identify them), to solicit comments and reviews.
 
+:1: footnote:[The Git Security mailing list: git-secur...@googlegroups.com]
+
+Patches which are security relevant should be submitted privately to
+the Git Security mailing list{1}.
+
 :1: footnote:[The current maintainer: gits...@pobox.com]
 :2: footnote:[The mailing list: git@vger.kernel.org]
 
-- 
2.17.0.921.gf22659ad46