kEnumName naming style is allowed or not?

185 views
Skip to first unread message

Kinuko Yasuda

unread,
Sep 9, 2012, 9:48:15 AM9/9/12
to chromium-dev
Hi chromium-dev folks,

For enum names I've been mainly using kEnumName style (as well as ENUM_NAME style depending on contexts), since I vaguely remember (or believed) that both styles are allowed.

But reviewing our coding style I'm bit confused.  It says:
"Though the Google C++ Style Guide says to use kConstantNaming for enums, Chrome still uses MACRO_STYLE naming for enums for consistency with our existing code."

This sounds a bit ambiguous.  Is using kEnumName style 1) allowed, 2) allowed but discouraged or 3) disallowed?  Should we be upset if we find kEnumNames?  Also maybe we should revise the phrase to make it more explicit?

Talking about our actual code base, we seem to be using more ENUM_NAME naming than kEnumName, though we could still see not small number of kEnumName usage throughout our code base.

Kinuko

Thiago Farina

unread,
Sep 9, 2012, 12:30:39 PM9/9/12
to kinuko...@google.com, chromium-dev
My view is that is better to use ENUM_NAME for enum constants (since
that is what they really are) but kConstantValue for "const int
kConstantValue = 10;", so when you see FOO_BAR you instantly knows it
comes from a enum type.

The FOO_BAR is a style that John pushed throughout in the content/
package, and I tried to do the same in ui/views.

--
Thiago

Darin Fisher

unread,
Sep 10, 2012, 12:34:56 PM9/10/12
to kinuko...@google.com, chromium-dev
The intent is for Chromium to use MACRO_STYLE for enum names instead of kConstantStyle.  Chromium was initially developed when MACRO_STYLE was the rule in the Google C++ Style Guide, and we added that text to the Chromium style guide in an effort to avoid the inconsistency that would result from adopting kConstantStyle for enum names.  I agree that the Chromium style guide could be clearer about this being an unconditional MUST.

-Darin



Kinuko

--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev

Peter Kasting

unread,
Sep 10, 2012, 1:34:40 PM9/10/12
to da...@google.com, kinuko...@google.com, chromium-dev
On Mon, Sep 10, 2012 at 9:34 AM, Darin Fisher <da...@chromium.org> wrote:
The intent is for Chromium to use MACRO_STYLE for enum names instead of kConstantStyle.  Chromium was initially developed when MACRO_STYLE was the rule in the Google C++ Style Guide, and we added that text to the Chromium style guide in an effort to avoid the inconsistency that would result from adopting kConstantStyle for enum names.  I agree that the Chromium style guide could be clearer about this being an unconditional MUST.

FWIW, I think our style guide is wrong here.  I don't think we should contradict the Google style guide, which is what we do now.  And I don't think our reasons for preserving this rule hold given that the Google style guide maintainers faced the same consistency arguments and elected to change anyway.

I think we should omit this portion of our style guide entirely and encourage kEnumName naming.  I don't think we have to go back and fix existing code (though I wouldn't stop people from doing so either).

PK 

Lei Zhang

unread,
Sep 10, 2012, 6:37:55 PM9/10/12
to pkas...@google.com, da...@google.com, kinuko...@google.com, chromium-dev
I just noticed a place where we are using kEnumName in the extensions
code. Likely someone only read the Google C++ Style Guide without
reading the Chromium Style Guide. Which ever way we decide, it would
be nice to have a presubmit check to help remind developers if they
use the wrong style.

Aaron Boodman

unread,
Sep 10, 2012, 9:03:24 PM9/10/12
to the...@chromium.org, Darin Fisher, Peter Kasting, chromium-dev, kinuko...@google.com

I've had people tell me in code reviews that both are allowed per style guide. The wording was pretty ambiguous before.

- a, via phone

James Robinson

unread,
Sep 10, 2012, 9:09:52 PM9/10/12
to a...@google.com, the...@chromium.org, Darin Fisher, Peter Kasting, chromium-dev, kinuko...@google.com
https://groups.google.com/a/chromium.org/d/msg/chromium-dev/U7GzRf_5F7s/pmVAjay5CwMJ proposed using SHOUTING_STYLE to be able to share enum names with java code, FWIW.

- James

Peter Kasting

unread,
Sep 11, 2012, 2:04:53 PM9/11/12
to Kinuko Yasuda, James Robinson, Aaron Boodman, Lei Zhang, Darin Fisher, chromium-dev, kinuko...@google.com
On Tue, Sep 11, 2012 at 2:30 AM, Kinuko Yasuda <kin...@google.com> wrote:
On Tue, Sep 11, 2012 at 10:09 AM, James Robinson <jam...@google.com> wrote:
https://groups.google.com/a/chromium.org/d/msg/chromium-dev/U7GzRf_5F7s/pmVAjay5CwMJ proposed using SHOUTING_STYLE to be able to share enum names with java code, FWIW.

Is the outcome of this discussion using SHOUTING_STYLE is actually beneficial?

I don't know that there is much outcome.  I've expressed an opinion that it isn't.  There haven't been many other clear opinions on whether we should do one, the other, or both, more just observations on what the existing code does or what the style guide says or has been interpreted to say.  Sounds like James found an argument for using it, but that thread sounds like it trailed off inconclusively.

PK

Ami Fischman

unread,
Sep 11, 2012, 3:16:11 PM9/11/12
to pkas...@google.com, Kinuko Yasuda, James Robinson, Aaron Boodman, Lei Zhang, Darin Fisher, chromium-dev, kinuko...@google.com
Just because it hasn't been said yet and the chromium style guide doesn't mention it: SHOUTING_STYLE was replaced by kCalmFriendly in google style because of conflicts with macro names.

-a (who agrees there isn't enough reason to diverge and hopes the chromium-style-guide entry is removed)

--

Darin Fisher

unread,
Sep 11, 2012, 3:22:30 PM9/11/12
to Peter Kasting, Kinuko Yasuda, James Robinson, Aaron Boodman, Lei Zhang, chromium-dev, kinuko...@google.com, Jonathan Dixon
+joth, to comment on whether or not clank ended up depending on MACRO_STYLE enum values.

Personally, I really dislike changes to the style guide because it means inconsistencies in the codebase.  I would love to have no differences between Chromium style and Google C++ style as that is certainly easier for everyone.  I'm just not sure that benefit is worth introducing inconsistencies to the codebase.  Afterall, the point of a style guide is to minimize inconsistencies.

-Darin

Jonathan Dixon

unread,
Sep 12, 2012, 11:21:51 PM9/12/12
to Darin Fisher, Peter Kasting, Kinuko Yasuda, James Robinson, Aaron Boodman, Lei Zhang, chromium-dev, kinuko...@google.com
On 11 September 2012 12:22, Darin Fisher <da...@chromium.org> wrote:
+joth, to comment on whether or not clank ended up depending on MACRO_STYLE enum values.


So in the case we were looking at, the net_errors_list.h is already very much using MARCO_STYLE anyway, so was indeed easy to rely on this in the build step the generates the java-side copy of that file (as illustrated by its usage in this basic test)

I can only imagine we will add more code that uses the same trick, where it makes sense.  e.g. NavigationController::LoadURLType would be one candidate. Right now we use not to pretty code to copy the constant values across from native to java on startup (see initializeConstants here

But regardless of the plumbing to achieve it, MARCO_STYLE allows us to have a consistent name for a given constant which is aids tracing call paths across the boundary (and being cross-language, grep-friendliness helps a lot)

Peter Kasting

unread,
Sep 13, 2012, 1:36:51 PM9/13/12
to Jonathan Dixon, Darin Fisher, Kinuko Yasuda, James Robinson, Aaron Boodman, Lei Zhang, chromium-dev, kinuko...@google.com
On Wed, Sep 12, 2012 at 8:21 PM, Jonathan Dixon <jo...@chromium.org> wrote:
So in the case we were looking at, the net_errors_list.h is already very much using MARCO_STYLE anyway, so was indeed easy to rely on this in the build step the generates the java-side copy of that file (as illustrated by its usage in this basic test)

I can only imagine we will add more code that uses the same trick, where it makes sense.  e.g. NavigationController::LoadURLType would be one candidate. Right now we use not to pretty code to copy the constant values across from native to java on startup (see initializeConstants here

But regardless of the plumbing to achieve it, MARCO_STYLE allows us to have a consistent name for a given constant which is aids tracing call paths across the boundary (and being cross-language, grep-friendliness helps a lot)

MARCO_STYLE, huh?  Exploring China are we?  :)

More seriously -- this is useful information and a good argument for keeping our current behavior.

PK 
Reply all
Reply to author
Forward
0 new messages