[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 20/63] pc87312: Rename TYPE_PC87312_SUPERIO to TYPE_PC87312
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH 20/63] pc87312: Rename TYPE_PC87312_SUPERIO to TYPE_PC87312 |
Date: |
Thu, 3 Sep 2020 21:55:03 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.11.0 |
On 9/3/20 7:22 PM, Eduardo Habkost wrote:
> 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.
Understood, no problem.