GNU bug report logs - #57400
29.0.50; Support sending patches from VC directly

Previous Next

Full log

View this message in rfc822 format

From: Eli Zaretskii <eliz <at> gnu.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: rpluim <at> gmail.com, 57400 <at> debbugs.gnu.org, ane <at> iki.fi, juri <at> linkov.net
Subject: bug#57400: 29.0.50; Support sending patches from VC directly
Date: Fri, 14 Oct 2022 09:50:38 +0300

> From: Philip Kaludercic <philipk <at> posteo.net>
> Cc: juri <at> linkov.net,  rpluim <at> gmail.com,  57400 <at> debbugs.gnu.org,  ane <at> iki.fi
> Date: Thu, 13 Oct 2022 22:05:52 +0000
> 
> > Also, there's a passive tense here...
> 
> (This is getting embarrassing...)

No need, it takes time to develop sensitivity to this stuff, and all
of us, including myself, make these mistakes from time to time.

> I've rewritten the entire thing, how does this sound:
> 
>   "Translate COMMIT string into symbolic form.
> A symbolic form is any revision that is not expressed in using
> SHA-1 object name.  If the optional argument FORCE is non-nil,
> this might include revision specifications like \"master~8\" (the
> 8th parent of the commit that \"master\" is currently pointing
> to).  If it is not possible to determine a symbolic commit, the
> function returns nil."

This is much better, but still leaves some obvious questions
unanswered.

>   Translate COMMIT string into symbolic form.

What is a "COMMIT string"?  I guess you mean the commit ID, and the
"string" part is just to indicate that its Lisp data type is a string?
If so, we usually say something like

  Translate Git revision descriptor of COMMIT, a string, to a symbolic form.

>                     If the optional argument FORCE is non-nil,
> this might include revision specifications like \"master~8\" (the
> 8th parent of the commit that \"master\" is currently pointing
> to).

This begs the question: what kind of COMMIT strings are acceptable if
FORCE is nil or omitted?  If we only accept SHA-1 hashes then, this
should perhaps be mentioned in the first sentence.  But from reading
the (unhelpful) man page of "git name-rev" (which leads down the
rabbit hole to "git rev-parse"), it is my understanding that this will
accept _any_ revision descriptor in any form.  So now I wonder why
accepting something like "master~8" needs a special knob: it's just
one of the forms supported by "git name-rev", isn't it?  So maybe you
don't even need the additional argument and don't have to document it?

But if there _are_ valid reasons not to accept the likes of
"master~8", they should be in the doc string.  For example:

  By default, COMMIT strings of the form "master~8" are rejected,
  because <describe the reason here>, but if FORCE is non-nil, they
  are allowed.

If "master~8" (or in general SOMETHING~N) is not the only form that is
rejected, the description should include the other forms as well,
perhaps as examples.

(And note that the text I proposed above fixed yet another
inconsistency: COMMIT is first described as a "revision" and "string"
and then as "revision specification"; a good doc string should use the
same consistent terminology throughout.)

>       If it is not possible to determine a symbolic commit, the
> function returns nil.

Again, for consistency, it is better to say

  If it is not possible to determine the symbolic form of COMMIT, the
  function returns nil.

because the first sentence talked about converting COMMIT into a
symbolic form.

Thanks, and keep up the good work.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.