Re: How to submit patches and patchsets via grub-devel

[Eli Schwartz]

On 4/23/20 10:20 AM, Hans Ulrich Niedermann wrote:
> Hello,
> 
> as I am continuing to flood this mailing list with patches, I am
> realizing that I am missing some general rules for how things work on
> grub-devel. Sorry for the inconvenience caused by that.
> 
> Anyway, here are a few questions I am beginning realize I should know
> the answers to before posting patches to grub-devel:
> 
>   * What to put into the cover letter (git send-email --compose)?
> 
>   * How to format commit messages?
> 
>   * What about those special lines within commit messages?
> 
>       * Signed-off-by:
>       * Reviewed-by:
> 
>     Who adds these special lines and when? If Daniel replies to me
>     posting a patch with "Reviewed-by: Daniel", does that mean I should
>     include that in my next iteration of that patch? Does it mean
>     Daniel approves of the patch and plans to merge it to master
>     himself with or without adding the Reviewed-by by himself?
> 
>     Are there other special lines I need to be aware of?

https://git.wiki.kernel.org/index.php/CommitMessageConventions#Trailers

Different projects mean different things, but Signed-off-by is a common
one:
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin

Reviewed-by is another convention originating in the kernel -- it
indicates Daniel thinks the patch is good, and the person (in this case
himself) who takes that patch and commits it to the project (or to the
kernel subsystem for later merging into torvalds/linux.git) will include
that line when committing -- there's no need to submit a v2/v3/etc patch
with just this modification to the commit message.

>   * How are reviews done? Is there a formal definition of preconditions
>     which must be satisfied before something is actually committed to
>     the master branch?

I'm not entirely sure what you're asking, but I'd guess "one or more
project maintainers will reply to the patch, either approving the patch
or asking for changes". But of course they're unlikely to approve a
non-documentation patch without giving it a trial run and seeing it work.

Is your question about what type of testing is done?

> I presume a good place to write those down for grub would be
> docs/grub-dev.texi. I had not intended to touch grub-dev.texi much
> before 2.06, but apparently this needs to be done first so I can
> properly do the other things for 2.06. Am I wrong in this presumption
> and just missing the place where the general rules on how to contribute
> patches are written down?

I think it's more or less:

- Some of these rules are passed along by word of mouth due to being de
  facto rules, and, while preferred, aren't hard requirements.

- Some of these rules are commonly used in various projects, so people
  automatically assume everyone knows them, even though they don't. Then
  when someone asks "but what are the rules, anyway" there's a mad
  scramble to hunt down a description in someone else's documentation,
  often from kernel.org

- Often, documentation doesn't get written down because it's obvious
  to the people who write documentation, and the people who need the
  docs don't know it. And in the case of procedural rules this is even
  more egregious, because you don't have code to point at to prove that
  it isn't documented. Then eventually someone who was unfamiliar with
  the process says "how about we document this" and everyone else is
  like "oh uh, yeah, that makes sense, we probably should have done this
  a long time ago".

It's always nice to find a documentation file such as
'doc/submitting-patches.asciidoc' (or reStructuredText or texinfo or
whatever the project uses) documenting the conventions, as well as any
project-specific guidance. It will also usually tell you the email
address for the appropriate mailing list. One example can be found here:

https://www.archlinux.org/pacman/submitting-patches.html

source:
https://git.archlinux.org/pacman.git/tree/doc/submitting-patches.asciidoc

Currently the grub-dev manual talks a lot about the coding style
(similar in spirit to my HACKING.asciidoc/HACKING.html) and very little
about how to submit patches. This information is definitely missing and
I believe a patch to update it would be very appropriate.

-- 
Eli Schwartz
Arch Linux Bug Wrangler and Trusted User

signature.asc
Description: OpenPGP digital signature