Daniel and Eli, thank you for nice explanations. I would like to add just a thew things...
On Thu, Apr 23, 2020 at 11:17:40AM -0400, Eli Schwartz wrote: > 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. Exactly! I would just add that I usually do not commit the patches immediately. I do it to leave some time for others to take a look and chime in if they find any issue. I usually commit a bunch of patches once per week. I try to not delay patches which fix grave bugs. > > * 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 Everybody who feels that a given area is in his/her expertise can review/test patches. However, final word belongs to the maintainers, in this case me, Vladimir and Alex. > non-documentation patch without giving it a trial run and seeing it work. I reject all patches witch breaks anything in the GRUB. Usually I tell what is broken... > 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. Yeah, sadly that is missing. We have to improve that thing for sure. Daniel _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
