Subject: Re: bug#57400: 29.0.50; Support sending patches from VC directly

From: Philip Kaludercic <[email-masked]>
Date: Thu, 13 Oct 2022 22:05:52 +0000

Eli Zaretskii <[email-masked]> writes:

>> Cc: Robert Pluim <[email-masked]>, [email-masked],
>>  Antoine Kalmbach <[email-masked]>
>> From: Philip Kaludercic <[email-masked]>
>> Date: Thu, 13 Oct 2022 20:25:03 +0000
>> 
>> -(defun vc-git-symbolic-commit (commit)
>> +(defun vc-git-symbolic-commit (commit &optional force)
>>    "Translate COMMIT string into symbolic form.
>> -Returns nil if not possible."
>> +Returns nil if not possible.
>
> And if possible?  I guess the first sentence lacks something it should
> say about that?  (Also, we use "Return", not "Returns", to be
> consistent with "Translate".)
>
>>                               If the optional argument FORCE is
>> +non-nil, revisions containing positional
>> +arguments (e.g. \"master~8\") will also be accepted."
>
> It is not a good idea to use "also" when you didn't explain before
> that what this does without FORCE.  (Maybe this should be part of the
> explanation of what you mean by "not possible" above?)
>
> Also, there's a passive tense here...

(This is getting embarrassing...)

> And finally, you use "positional arguments", but "man gitrevisions"
> doesn't use this term at all.  So maybe we should use a more accepted
> terminology, or at least provide more than just one example to explain
> that via the examples?
>
> Thanks.

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."