qemu-devel
[Top][All Lists]
Advanced

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

Re: [PATCH 1/2] docs: Fix botched rST conversion of 'submitting-a-patch.


From: Kashyap Chamarthy
Subject: Re: [PATCH 1/2] docs: Fix botched rST conversion of 'submitting-a-patch.rst'
Date: Mon, 22 Nov 2021 11:20:23 +0100

On Mon, Nov 22, 2021 at 10:51:15AM +0100, Thomas Huth wrote:
> On 19/11/2021 20.31, Kashyap Chamarthy wrote:
> > I completely botched up the merged[0] rST conversion of this document by
> > accidentally dropping entire hunks (!) of text. :-(  I made it very hard
> > for reviewers to spot it, as the omitted text was buried deep in the
> > document.  To fix my hatchet job, I reconverted the "SubmitAPatch"
> > wiki[1] page from scratch and replaced the existing rST with it, while
> > making sure I incorporated previous feedback.
> 
> Thanks for the quick update! I've now tripple-checked the differences
> between the old wiki page and the new rst page, and I think with some
> additional small changes on top (which I will do while picking up your
> patch, see below), we should be fine now.

Thanks for your eagle eyes.

[...]

> > -- `QEMU Coding Style
> > -  <https://qemu-project.gitlab.io/qemu/devel/style.html>`__
> > -
> > +-  `QEMU Coding Style 
> > <https://qemu-project.gitlab.io/qemu/devel/style.html>`__
> 
> While we're at it, I'll replace the link with an internal link when picking
> up this patch (so that it can also be used in offline documentation).

Sure.

> >   -  `Automate a checkpatch run on
> > -   commit 
> > <http://blog.vmsplice.net/2011/03/how-to-automatically-run-checkpatchpl.html>`__
> > +   commit 
> > <https://blog.vmsplice.net/2011/03/how-to-automatically-run-checkpatchpl.html>`__
> > +-  Spell check your patches
> 
> I'll add the link to https://wiki.qemu.org/Contribute/SpellCheck again.

Please don't -- I intentionally dropped it in a previous iteration based
on this feedback from Peter Maydell:

    "I'm not sure that SpellCheck in particular is sufficiently baked
    to be in the actual docs. I'd rather just drop the reference to it
    from SubmitAPatch."
    
    (https://lists.nongnu.org/archive/html/qemu-devel/2021-11/msg00137.html)

> > @@ -85,7 +91,7 @@ Split up longer patches into a patch series of logical 
> > code changes.
> >   Each change should compile and execute successfully. For instance, don't
> >   add a file to the makefile in patch one and then add the file itself in
> >   patch two. (This rule is here so that people can later use tools like
> > -`git bisect <http://git-scm.com/docs/git-bisect>`__ without hitting
> > +```git bisect`` <http://git-scm.com/docs/git-bisect>`__ without hitting
> 
> You've re-introduced the broken git-bisect link. I'll fix it again.

Oops; thanks!  It's hard to spot these double-ticks.

> > @@ -170,11 +175,74 @@ displays the subject line some distance apart (that 
> > is, a body that
> >   starts with "... so that" as a continuation of the subject line is
> >   harder to follow).
> > +If your patch fixes a commit that is already in the repository, please
> > +add a line with "Fixes: ("Fixed commit subject")" below the patch
> 
> Missing the "<at-least-12-digits-of-SHA-commit-id>" from the Wiki page. I'll
> add it.
>
> > +description / before your "Signed-off-by:" line in the commit message.
> > +
> > +If your patch fixes a bug in the gitlab bug tracker, please add a line
> > +with "Resolves: " to the commit message, too. Gitlab can close bugs
> 
> Missing the "<URL-of-the-bug>" from the Wiki. I'll add it.

Uh, not sure how I missed the above two bits.

> > +automatically once commits with the "Resolved:" keyword get merged into
> > +the master branch of the project. And if your patch addresses a bug in
> > +another public bug tracker, you can also use a line with "Buglink: " for
> 
> dito.
> 
> > @@ -223,13 +291,22 @@ use --numbered so the cover and the patch have 
> > distinct subject lines).
> 
> I noticed that the --numbered will be translated into –numbered, so I'll add
> some `` quotes here.

Good catch; my eyes glazed over it.

> > @@ -288,6 +370,18 @@ it's best to:
> >      of the patchset you're looking for review on, and why reviewers
> >      should care
> > +.. _consider_whether_your_patch_is_applicable_for_stable:
> > +
> > +Consider whether your patch is applicable for stable
> > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > +
> > +If your patch fixes a severe issue or a regression, it may be applicable
> > +for stable. In that case, consider adding ``Cc: qemu-stable@nongnu.org``
> > +to your patch to notify the stable maintainers.
> > +
> > +For more details on how QEMU's stable process works, refer to
> > +`docs/devel/stable-process.rst 
> > <https://git.qemu.org/?p=qemu.git;a=blob_plain;f=docs/devel/stable-process.rst;hb=HEAD>`__.
> 
> While at this, I'll also replace this external link with an internal one.

[...]

> >   your patch in a particular staging branch. Periodically, the maintainer
> >   then sends a `pull request
> >   
> > <https://qemu-project.gitlab.io/qemu/devel/submitting-a-pull-request.html>`__
> 
> I'll also use an internal link for this reference here.

Sure.

Many thanks for the careful combing!

[...]

-- 
/kashyap




reply via email to

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