Re: [PATCH 20/63] pc87312: Rename TYPE_PC87312_SUPERIO to TYPE_PC87312

[Eduardo Habkost]

On Thu, Sep 03, 2020 at 12:16:47PM -0400, Eduardo Habkost wrote:
> On Thu, Sep 03, 2020 at 02:45:12PM +0200, Philippe Mathieu-Daudé wrote:
> > On 9/3/20 12:42 AM, Eduardo Habkost wrote:
> > > This will make the type name constant consistent with the name of
> > > the type checking macro.
> > > 
> > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > > ---
> > > Cc: "Hervé Poussineau" <hpoussin@reactos.org>
> > > Cc: qemu-ppc@nongnu.org
> > > Cc: qemu-devel@nongnu.org
> > > ---
> > >  include/hw/isa/pc87312.h | 4 ++--
> > >  hw/isa/pc87312.c         | 2 +-
> > >  2 files changed, 3 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/include/hw/isa/pc87312.h b/include/hw/isa/pc87312.h
> > > index a65168a157..da8dc5ddf5 100644
> > > --- a/include/hw/isa/pc87312.h
> > > +++ b/include/hw/isa/pc87312.h
> > > @@ -29,10 +29,10 @@
> > >  #include "qom/object.h"
> > >  
> > >  
> > > -#define TYPE_PC87312_SUPERIO "pc87312"
> > > +#define TYPE_PC87312 "pc87312"
> > 
> > We loose self-documentation. What is a TYPE_PC87312
> > when reviewing a board setup code? Should we add a
> > comment /* Create the Super I/O */? The current name
> > is self-describing...

I've just realized that TYPE_PC87312_SUPERIO is not used anywhere
in the code, so I don't understand where exactly this comment
applies.

> > 
> > Is it easier to rename the type as 'pc87312-superio'?
> 
> This is an option.  In that case, I would like to rename the
> PC87312 type checking macro to PC87312_SUPERIO, if that's OK.
> 
> The actual string name doesn't matter for the QOM macros, by the
> way.  We can still rename it if you want to, but we don't have
> to.

Based on Daniel's suggestion of keeping the macro names
consistent with the QOM type name string, I'd like to keep the
original color of the bike shed and keep this patch as is.

I will queue this patch on machine-next with Hervé's Reviewed-by
line.

If anybody wants to rename the user-visible QOM type name string
later, that's OK.  But I don't think this should be done as part
of the QOM boilerplate cleanup work.

-- 
Eduardo