qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [RFC] spapr: Use CamelCase properly


From: David Gibson
Subject: Re: [Qemu-devel] [RFC] spapr: Use CamelCase properly
Date: Thu, 7 Mar 2019 16:21:49 +1100
User-agent: Mutt/1.11.3 (2019-02-01)

On Wed, Mar 06, 2019 at 08:22:51AM +0100, Cédric Le Goater wrote:
> Hello,
> 
> On 3/6/19 5:38 AM, David Gibson wrote:
> > The qemu coding standard is to use CamelCase for type and structure names,
> > and the pseries code follows that... sort of.  There are quite a lot of
> > places where we bend the rules in order to preserve the capitalization of
> > internal acronyms like "PHB", "TCE", "DIMM" and most commonly "sPAPR".
> > 
> > That was a bad idea - it frequently leads to names ending up with
> > unreadable clusters of capital letters, and means they don't appear to the
> > eye as type identifiers, which is kind of the point of the CamelCase
> > convention in the first place.
> > 
> > In short, keeping type identifiers look like CamelCase is more important
> > than preserving standard capitalization of internal "words".  So, this
> > patch renames a heap of spapr internal type names to a more standard
> > CamelCase.
> 
> +1
>  
> > We also rename VIOsPAPR* to SpaprVio*, the less natural reverse ordering
> > was only there in the first place to mitigate the capital-letter cluster
> > of "sPAPRVIO".
> 
> Could we remove the Device prefix ? I don't find it very useful.

[Aside: ITYM "suffix"]

> 
>       SpaprVioVtyDevice
>       SpaprVioVlanDevice

Done.

>       SpaprVioDevice

I'll leave this one.  "SpaprVio" doesn't really make grammatical
sense, and "SpaprVioDevice" works in analogy with, say, PCIDevice.

> 
> > This is 100% a mechanical search-and-replace patch.  It will, however,
> > conflict with essentially any and all outstanding patches touching the
> > spapr code.
> > 
> > Signed-off-by: David Gibson <address@hidden>
> 
> I have spotted dromedaries :
> 
>       typedef struct SpaprDRConnector

I've changed this to SpaprDrc.  Shorter and makes it clearer that
these are the same things as the "DRC"s we reference elsewhere.

>       typedef struct SpaprDRCPhysical

Changed to SpaprDrcPhysical

> 
> Could we get rid of the 'State' prefix ? It does not add much to the name
> and usually the associated macros remove it.

True, but it is a common qemu pattern, so I think I'll leave these for now.

>       SpaprRngState
>       SpaprPhbState
>       SpaprRtcState
>       SpaprDimmState
>       SpaprMachineState
>       SpaprCpuState
> 
> It would be good to reverse the xics filenames also.

I'm not really sure what you mean by that, and in any case I don't
think it's really in scope for this patch.

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

Attachment: signature.asc
Description: PGP signature


reply via email to

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