Re: [PATCH lbwww] Better instructions to create patches

[Denis 'GNUtoo' Carikli]

On Wed, 22 Mar 2023 19:51:16 +0100
Adrien Bourmault <neox@a-lec.org> wrote:

> Signed-off-by: Adrien Bourmault <neox@a-lec.org>
> ---
>  site/git.md | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/site/git.md b/site/git.md
> index 4de7ce3..8490066 100644
> --- a/site/git.md
> +++ b/site/git.md
> @@ -145,7 +145,7 @@ non-free.
>  GNU+Linux is generally recommended as the OS of choice, for Libreboot
>  development. However, BSD operating systems also boot on Libreboot
> machines. 
> -Send patches
> +Send patches & contribute
Very good idea.

>  You can submit your patches to the 
> @@ -156,6 +156,14 @@ A simple guide to properly configure your git
> installation to send emails has been made by
> [sourcehut](https://git-send-email.io/) or you can use the [sourcehut
> interface](https://man.sr.ht/git.sr.ht/#sending-patches-upstream) to
> create patches. +Since the mailing list accepts patches for all the
> repositories we manage, please specify which repository your patch is
> for. To do this, you'll need to use: +
> +     git config format.subjectPrefix "PATCH name_of_the_repo"
There is a way to improve it slightly.

If we use that it will then look like that:
> Subject: [PATCH lbwww v2 1/2] Fix some typo in the documentation> 

If we use:
> git config format.subjectPrefix 'PATCH][lbwww]['

We end up with:
> Subject: [PATCH][lbwww][ v2 1/2] Fix some typo in the documentation> 

So we need to have the lbwww before because there is a space before
v2, so it means that it's not meant to be used like that. 

With:
> git config format.subjectPrefix 'lbwww] [PATCH'
That then looks like that:
> Subject: [lbwww] [PATCH v2 1/2] Fix some typo in the documentation>

That looks way better: we have some spacing and things are more clear,
and also inline with the use case envisioned by git.

And other projects seems to also put the extra info before [PATCH]:
https://www.openembedded.org/wiki/How_to_submit_a_patch_to_OpenEmbedded#Backporting_fixes_to_stable_releases

I think it's important as having something as clear as possible (and
similar to other projects usage) as it will help not missing patches in
the long run.

> +Please also sign-off your patches, which you can configure with:
> +
> +     git config format.signOff yes

This is also a good idea, but to work we also need to inform people
of the consequence of adding that signed-off-by. Linux uses the
"Developer's Certificate of Origin 1.1" that is available in its
documentation[1].

Guix uses signed-off by for a completely different purpose (it uses it
like Acked-by).

There are also interesting things in use in Linux that are
mentioned in the same document like Reported-by, Tested-by,
Reviewed-by, so we could probably point to that document for other uses
cases as well (not necessarily in this patch though, as this is more
for maintainers and users reading existing commit messages).

Also note that not everything in that document applies to our use case
as we don't have C code for instance, so C coding style don't matter.

As for using a coding style, in practice it helps a lot for code
readability and easy patch rebasing, but:
(1) I don't know any coding style for shell
(2) in practice I found it really hard to enforce something like that
    without automatic tools that can check it, and here I don't know any
    for shell.
(3) If we have some legacy code base, we'd need to do a painful
    conversion to a new code style.

I've put all I know in a wiki[2], so it probably needs some more
research for shell. There are probably stuff around but we'd probably
need to do more research to find good standards and tools.

Though for contributions we could probably at least try to ask people
to limit the line length to 80 characters whenever possible and try to
stay consistent with the rest of the file with regard to tap and spaces.

All that doesn't necessarily need to be in the same patch though.

References:
-----------
[1]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.3-rc3#n364
[2]https://redmine.replicant.us/projects/upstreaming/wiki/Upstream#Coding-standards-and-style

Denis.

pgpBX9nLsOWJh.pgp
Description: OpenPGP digital signature