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

DoneSubmitted by zimoun.
Details
3 participants
  • Oleg Pykhalov
  • Ludovic Courtès
  • zimoun
Owner
unassigned
Severity
normal
Z
Z
zimoun wrote on 12 Oct 2020 10:20
(address . guix-patches@gnu.org)(name . zimoun)(address . zimon.toutoune@gmail.com)
20201012082003.19936-1-zimon.toutoune@gmail.com
* doc/contributing.texi (Submitting Patches): Add item about 'git-format-patch--base'.--- doc/contributing.texi | 6 ++++++ 1 file changed, 6 insertions(+)
Toggle diff (21 lines)diff --git a/doc/contributing.texi b/doc/contributing.texiindex 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.+ @item If the authors of the packaged software provide a cryptographic signature for the release tarball, make an effort to verify the
base-commit: e746d30ec15852ec50e4247ea0a07ab2a9a69aa8-- 2.28.0
O
O
Oleg Pykhalov wrote on 24 Oct 2020 11:01
(name . zimoun)(address . zimon.toutoune@gmail.com)(address . 43946-done@debbugs.gnu.org)
87d0189ev4.fsf@gmail.com
Hi,
zimoun <zimon.toutoune@gmail.com> writes:
Toggle quote (20 lines)> * 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.
“git format-patch” already mentioned in the documentation in the firstparagraph https://guix.gnu.org/manual/en/html_node/Submitting-Patches.html
Do I miss something? Fill free to reopen the issue ;-)
Oleg.
-----BEGIN PGP SIGNATURE-----
iQJIBAEBCgAyFiEEcjhxI46s62NFSFhXFn+OpQAa+pwFAl+T7X8UHGdvLndpZ3VzdEBnbWFpbC5jb20ACgkQFn+OpQAa+pyudBAArBS4ldmCP+57vXlefi6pc75gWORpEQRlqggeykpj8wxAxybD4cjYyLTiZuoDzxMeusnm/QoW1efEo9QYYKXb7V2z0JJIaN3u2nSN5dOmzDYJZZ1cXZCiCcOKPuTctE+lm/A+6dOJnRjWsvI3j3MK+uaGRyeYLLgXr8Fdp7QD/SLUNYJh+dOnx5LwA/nohyrjJkH+f1a2sCeEQrcdJz+FRKgF/nK7mm143NWEAOvOxBa252K1NtGB9FF6KzjewHVyh/9O+8bDBoNoFSGgeMcn1FeDxpli2ecX7b+7ndLbDuw2kO69cGxSR3iz8YOA2Va71CnFBntmWqMouegAuOL7BNdj4ZvPe3QPd2u8fl0hcrBEXFSbmiKygODGN/wL50Kp5WguF917LoxWo9hy6qNSUP6LoDjeNC7olHR+iRK2PZt/BhRwDTZ2KYuJlrzn4I5nyA9geUtrvCYtWxbfsi51afOZARML8bHJgLeNjlSyHOUKgOmPj2n1gO1VxMoJKrXdehjY9vf15FJOyT5TIcJFYoqONZWfmky7X5R8pKWQItfD8uiQdaxukFTMh7VzFJsIqYw5UEzO4wn1noQA8g/ayhJmsnwA6pOVujOi33NzouP4ShMuL0FeBOopUE4FHsSyqPwNSyCozm2QIiBm0LZLN9U6hCcnejWEBrszF1aHiIA==Tcqj-----END PGP SIGNATURE-----
Closed
Z
Z
zimoun wrote on 24 Oct 2020 11:17
(name . Oleg Pykhalov)(address . go.wigust@gmail.com)(address . 43946-done@debbugs.gnu.org)
CAJ3okZ0ANAfyWVbgUEe1Ryv35C9OA7CS-4bx_ey5YTmisi_tPQ@mail.gmail.com
Dear,
Thank you for the review.
On Sat, 24 Oct 2020 at 11:01, Oleg Pykhalov <go.wigust@gmail.com> wrote:
Toggle quote (25 lines)> zimoun <zimon.toutoune@gmail.com> writes:>> > * 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.>> “git format-patch” already mentioned in the documentation in the first> paragraph https://guix.gnu.org/manual/en/html_node/Submitting-Patches.html
The point of the patch is: - the addition of the cindex - the addition of the option --baseand AFAITC it is not already in the manual. If it is the wrong place,please let me know where it could be better.
All the best,simon
Closed
L
L
Ludovic Courtès wrote on 15 Jan 14:30 +0100
Re: bug#43946: [PATCH] doc: Add item to "Submitting Patches" section.
(name . zimoun)(address . zimon.toutoune@gmail.com)(address . 43946@debbugs.gnu.org)
87zh1axr7c.fsf@gnu.org
Hi Simon!
zimoun <zimon.toutoune@gmail.com> skribis:
Toggle quote (20 lines)> * 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 acouple 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 inthis case.
Closing? :-)
Thanks,Ludo’.
Z
Z
zimoun wrote on 15 Jan 15:00 +0100
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 43946@debbugs.gnu.org)
CAJ3okZ316hbOsXJ3R8UCpEs--yqdVaw8pm3qp0p138JTDox6-w@mail.gmail.com
Hi Ludo,
On Fri, 15 Jan 2021 at 14:30, Ludovic Courtès <ludo@gnu.org> wrote:
Toggle quote (8 lines)> > +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.
I am not buying your argument. :-)It is not because <name> does not use this information that it cannotbe a recommendation, i.e., a suggestion or advice on what seemshelpful.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 patchapplies 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.rst2: https://docs.kyleam.com/piem/Using-b4-to-apply-patches.html3: http://issues.guix.gnu.org/446254: https://inbox.kyleam.com/piem/20201115061518.22191-1-kyle@kyleam.com/
Toggle quote (2 lines)> Closing? :-)
As you want. :-)But, I think "qui peut le plus peut le moins". Even if maybe mywording is not the good one, I still think that the "base-commit"where the patch applies should be provided, at least suggested.
Cheers,simon
Z
Z
zimoun wrote on 26 Mar 06:40 +0100
Re: [bug#43946] [PATCH] doc: Add item to "Submitting Patches" section.
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 43946@debbugs.gnu.org)
867dlucw1e.fsf@gmail.com
Hi Ludo,

On Fri, 15 Jan 2021 at 15:00, zimoun <zimon.toutoune@gmail.com> wrote:
Toggle quote (10 lines)> On Fri, 15 Jan 2021 at 14:30, Ludovic Courtès <ludo@gnu.org> wrote:>>> > +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.
[...]
Toggle quote (7 lines)>> Closing? :-)>> As you want. :-)> But, I think "qui peut le plus peut le moins". Even if maybe my> wording is not the good one, I still think that the "base-commit"> where the patch applies should be provided, at least suggested.
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 ifthey are. Otherwise, the patch is useless or it needs to be rewrittenby hand (or please indicate me how to do :-)).
BTW, it helps automation tools.
Sometimes, this base-commit is useless for the reviewer workflow buthaving it does not interfere. Having an information does not mean itmust be used. However, not having an information implies it cannot beused. ;-)

Cheers,simon
Z
Z
zimoun wrote on 21 Sep 11:53 +0200
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 43946@debbugs.gnu.org)
8635pyb6fe.fsf@gmail.com
Hi Ludo,
On Fri, 15 Jan 2021 at 14:30, Ludovic Courtès <ludo@gnu.org> wrote:
Toggle quote (27 lines)> 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:
Toggle snippet (35 lines)It is not because <name> does not use this information that it cannot bea recommendation, i.e., a suggestion or advice on what seems helpful.Other said, it is not because it is "useless in this case" that itisuseless in other cases.
For example, this information about which known commit that patchapplies 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.rst2: https://docs.kyleam.com/piem/Using-b4-to-apply-patches.html3: http://issues.guix.gnu.org/446254: https://inbox.kyleam.com/piem/20201115061518.22191-1-kyle@kyleam.com/
[..]
I still think that recommending to provide the commit on which itisknown that the patch (or patch set) applies is a good recommendation.Especially when the submission rate is greater than the review rateandthe tree is moving really quickly (yeah!).
It is no extra work for the submitter and really helps for thereviewer. They applies at base-commit, checks, rebases and resolvesconflicts if they are. Otherwise, the patch is useless or it needs tobe 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 buthaving it does not interfere. Having an information does not mean itmust be used. However, not having an information implies it cannot beused. ;-)
#: 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 notapply anymore. It is not encouraging to review and decrease the queuewhen first the reviewer has to guess onto which commit the patchapplies.

Cheers,simon
L
L
Ludovic Courtès wrote on 22 Sep 15:02 +0200
Re: bug#43946: [PATCH] doc: Add item to "Submitting Patches" section.
(name . zimoun)(address . zimon.toutoune@gmail.com)(address . 43946-done@debbugs.gnu.org)
87bl4khif2.fsf_-_@gnu.org
Hi,
zimoun <zimon.toutoune@gmail.com> skribis:
Toggle quote (5 lines)> 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.
Yeah in hindsight I think I agree with you. I went ahead and (finally!)applied the patch.
Thanks for persevering, and apologies for my stubbornness! :-)
Ludo’.
Closed
?
Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send email to 43946@debbugs.gnu.org