guix-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[bug#43946] [PATCH] doc: Add item to "Submitting Patches" section.


From: zimoun
Subject: [bug#43946] [PATCH] doc: Add item to "Submitting Patches" section.
Date: Tue, 21 Sep 2021 11:53:57 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux)

Hi Ludo,

On Fri, 15 Jan 2021 at 14:30, Ludovic Courtès <ludo@gnu.org> wrote:
> zimoun <zimon.toutoune@gmail.com> skribis:
>
>> * doc/contributing.texi (Submitting Patches): Add item about 
>> 'git-format-patch
>> --base'.
>> ---
>>  doc/contributing.texi | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/doc/contributing.texi b/doc/contributing.texi
>> index af3601442e..5ea3cb1899 100644
>> --- a/doc/contributing.texi
>> +++ b/doc/contributing.texi
>> @@ -932,6 +932,12 @@ Before submitting a patch that adds or modifies a 
>> package definition,
>>  please run through this check list:
>>  
>>  @enumerate
>> +@cindex @code{git format-patch}
>> +@cindex @code{git-format-patch}
>> +@item
>> +We recommend to use the command @code{git format-patch --base} to
>> +include the commit where your patch applies.
>
> I’m not entirely convinced TBH, in part because I know I often pile a
> couple of WIP branches on top of one another, “knowing what I’m doing”
> (actually hoping that I do), and so the base commit would be useless in
> this case.

Could you explain more?  Here [#,@], I argument:

--8<---------------cut here---------------start------------->8---
It is not because <name> does not use this information that it cannot be
a recommendation, i.e., a suggestion or advice on what seems helpful.
Other said, it is not because it is "useless in this case" that it
isuseless in other cases.

For example, this information about which known commit that patch
applies is helping for the automation of testing patches.  Well,
see [1,2] for instance.  Discussions of such tooling happened in
#44625 [3] and Emacs helper [4].

1: https://git.kernel.org/pub/scm/utils/b4/b4.git/tree/README.rst
2: https://docs.kyleam.com/piem/Using-b4-to-apply-patches.html
3: http://issues.guix.gnu.org/44625
4: https://inbox.kyleam.com/piem/20201115061518.22191-1-kyle@kyleam.com/

[..]

I still think that recommending to provide the commit on which it
isknown that the patch (or patch set) applies is a good recommendation.
Especially when the submission rate is greater than the review rate
andthe tree is moving really quickly (yeah!).

It is no extra work for the submitter and really helps for the
reviewer.  They applies at base-commit, checks, rebases and resolves
conflicts if they are.  Otherwise, the patch is useless or it needs to
be rewritten by hand (or please indicate me how to do :-)).

BTW, it helps automation tools.

Sometimes, this base-commit is useless for the reviewer workflow but
having it does not interfere.  Having an information does not mean it
must be used.  However, not having an information implies it cannot be
used. ;-)
--8<---------------cut here---------------end--------------->8---

#: <http://issues.guix.gnu.org/issue/43946#5>
@: <http://issues.guix.gnu.org/issue/43946#6>


Seeing the number of patches in the patch tracker, most of them does not
apply anymore.  It is not encouraging to review and decrease the queue
when first the reviewer has to guess onto which commit the patch
applies.


Cheers,
simon





reply via email to

[Prev in Thread] Current Thread [Next in Thread]