Re: [PATCH 14/18] docs/system: Convert qemu-cpu-models.texi to rST

From: Kashyap Chamarthy <kchamart@redhat.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	QEMU Developers <qemu-devel@nongnu.org>
Subject: Re: [PATCH 14/18] docs/system: Convert qemu-cpu-models.texi to rST
Date: Thu, 27 Feb 2020 13:37:22 +0100	[thread overview]
Message-ID: <20200227123722.GN24572@paraplu> (raw)
In-Reply-To: <CAFEAcA-ps7dmo_7Lgf2kTsy0nvFgU6qopS8HvZMu3DY81VvE_g@mail.gmail.com>

On Thu, Feb 27, 2020 at 12:10:02PM +0000, Peter Maydell wrote:
> On Wed, 26 Feb 2020 at 11:30, Paolo Bonzini <pbonzini@redhat.com> wrote:

[...]

> >  delete mode 100644 docs/system/qemu-cpu-models.texi
> 
> Why don't we also delete cpu-models-x86.texi and
> cpu-models-mips.texi ?

We can; will do.

> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 0d3ee8bdc6..01d1ee4b50 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -320,7 +320,7 @@ F: tests/tcg/i386/
> >  F: tests/tcg/x86_64/
> >  F: hw/i386/
> >  F: disas/i386.c
> > -F: docs/system/cpu-models-x86.texi
> > +F: docs/system/cpu-models-x86.rst
> 
> This isn't the filename the patch actually uses.

Originally, I used: 

    -F: docs/qemu-cpu-models.texi
    +F: docs/system/qemu-cpu-models.rst

as part of my v4, here:
https://lists.nongnu.org/archive/html/qemu-devel/2020-02/msg07165.html

But got "obsoleted" with Paolo's split of x86 & MIPS change.

> There should be a corresponding new F: line for the mips version too.

Correct; will add.

> >  qemu.1: qemu-doc.texi qemu-options.texi qemu-monitor.texi qemu-monitor-info.texi
> >  qemu.1: docs/system/qemu-option-trace.texi
> > -docs/system/qemu-cpu-models.7: docs/system/qemu-cpu-models.texi docs/system/cpu-models-x86.texi docs/system/cpu-models-mips.texi
> >
> >  html: qemu-doc.html docs/interop/qemu-qmp-ref.html docs/interop/qemu-ga-ref.html sphinxdocs
> >  info: qemu-doc.info docs/interop/qemu-qmp-ref.info docs/interop/qemu-ga-ref.info
> 
> The install line for qemu-cpu-models.7 also needs updating to
> include $(MANUAL_BUILDDIR).
> 
> The distclean line that removes qemu-cpu-models.7 can be removed.

Noted, will do.

[...]

> >  # General information about the project.
> >  project = u'QEMU'
> > -copyright = u'2019, The QEMU Project Developers'
> > +copyright = u'2020, The QEMU Project Developers'
> >  author = u'The QEMU Project Developers'
> >
> >  # The version info for the project you're documenting, acts as replacement for
> 
> We should indeed fix the copyright date, but not in the middle
> of this texi->rst series, and we should update the QEMU_COPYRIGHT
> macro in qemu-common.h at the same time.

Indeed.  A voice at the back of my head _did_ nag, "you're mashing this
unrelated change into the middle of .texi --> .rst series..."

Will make it a separate patch.

> > +
> > +rst_prolog = '''
> > +.. include:: defs.rst.inc
> > +'''
> > +
> 
> Maybe we should do this in docs/conf.py where we update
> rst_epilog with the CONFDIR replacement ?

I'm ambivalent about it; can do it in docs/conf.py if you that is more
appropriate.

> Doing the "put these replacements into a generic defs.rst.inc"
> would also be clearer in its own patch, I think.

Fair enough.  

> >  # One entry per manual page. List of tuples
> >  # (source start file, name, description, authors, manual section).
> >  man_pages = [
> >      ('qemu-block-drivers', 'qemu-block-drivers',
> >       u'QEMU block drivers reference',
> > -     ['Fabrice Bellard and the QEMU Project developers'], 7)
> > +     ['Fabrice Bellard and the QEMU Project Developers'], 7),
> 
> If we want to capitalize the 'D' in Developers, we should
> make that fix in its own commit, and we should update
> the QEMU_COPYRIGHT macro too, for consistency.

I did wonder about that too; fair enough.  Will not mix it here.

> > +    ('qemu-cpu-models', 'qemu-cpu-models',
> > +     u'QEMU CPU Models',
> > +     ['The QEMU Project Developers'], 7)
> >  ]
> 
> I would mention explicitly in the commit message that Dan agreed
> to the updating of the manpage's Author information to
> the more generic string.

Can do; I actually mentioned it in my v4 revision history, below the
"---" line
(https://lists.nongnu.org/archive/html/qemu-devel/2020-02/msg07165.html):

   - Fix author attribution as per the thread:
     Message-ID:
     <CAFEAcA8dTR6xkQnsf32HAo6aDXf5Z0UmkhL3+257KWLj1ES6Sw@mail.gmail.com>
     [danpb, pm215]

Can spell it out as part of the commit message.

In summary, to-do for v5:

  - Fix the things you pointed out in this patch here.

  - Don't mix unrelated changes with .texi --> rst conversion (undo the
    copyright year and the capitalization changes; make them separate
    patches).

  - Do the split of x86 | MIPS in a separate patch, including the change
    of moving the macros to 'defs.rst.inc'.

-- 
/kashyap