Re: Coding style: Making the `e` prefix for enum variants not mandatory?

2018-07-07 Thread Emilio Cobos Álvarez




On 7/6/18 8:21 PM, Ehsan Akhgari wrote:

On Fri, Jun 29, 2018 at 2:36 PM, smaug  wrote:


On 06/29/2018 05:58 PM, Boris Zbarsky wrote:


On 6/29/18 10:30 AM, Nathan Froyd wrote:


Given the language-required qualification for
`enum class` and a more Rust-alike syntax, I would feel comfortable
with saying CamelCase `enum class` is the way to go.



For what it's worth, I agree.  The point of the "e" prefix is to
highlight that you have an enumeration and add some poor-man's namespacing
for a potentially large number of common-looking names, and the
language-required qualification already handles both of those.



That works if we're consistently naming static variables and such using
s-prefix etc, so that
class Foo1
{
   static int sBar;
};
Foo1::sBar

is clearly different to
enum class Foo2
{
   Bar
};

Foo2::Bar


(personally I'd still prefer using e-prefix always with enums, class or
not. Foo1::sBar vs Foo2::eBar is easier to distinguish than Foo1::sBar vs
Foo2::Bar)



Looking at other popular style guidelines, the Google C++ style requires
the 'k' prefix on both enum and enum class <
https://google.github.io/styleguide/cppguide.html#Enumerator_Names>
presumably because it doesn't require any special prefix for static
members.  But given that ours does, it seems that in the case of enum
classes, not using a prefix should be acceptable.

Based on Nathan's analysis (thanks, Nathan!) and the previous thread, it
seems like Emilio's edit should be reverted and a note should be added
about the usage of CamelCase for enum classes.  Emilio, do you mind doing
that, please?


Sure, just did:


https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style$compare?locale=en-US&to=1394287&from=1392024

Feel free to clarify or change as you think it's more useful.

Thanks Ehsan!

 -- Emilio
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Coding style: Making the `e` prefix for enum variants not mandatory?

2018-07-06 Thread Ehsan Akhgari
On Fri, Jun 29, 2018 at 2:36 PM, smaug  wrote:

> On 06/29/2018 05:58 PM, Boris Zbarsky wrote:
>
>> On 6/29/18 10:30 AM, Nathan Froyd wrote:
>>
>>> Given the language-required qualification for
>>> `enum class` and a more Rust-alike syntax, I would feel comfortable
>>> with saying CamelCase `enum class` is the way to go.
>>>
>>
>> For what it's worth, I agree.  The point of the "e" prefix is to
>> highlight that you have an enumeration and add some poor-man's namespacing
>> for a potentially large number of common-looking names, and the
>> language-required qualification already handles both of those.
>>
>>
> That works if we're consistently naming static variables and such using
> s-prefix etc, so that
> class Foo1
> {
>   static int sBar;
> };
> Foo1::sBar
>
> is clearly different to
> enum class Foo2
> {
>   Bar
> };
>
> Foo2::Bar
>
>
> (personally I'd still prefer using e-prefix always with enums, class or
> not. Foo1::sBar vs Foo2::eBar is easier to distinguish than Foo1::sBar vs
> Foo2::Bar)
>

Looking at other popular style guidelines, the Google C++ style requires
the 'k' prefix on both enum and enum class <
https://google.github.io/styleguide/cppguide.html#Enumerator_Names>
presumably because it doesn't require any special prefix for static
members.  But given that ours does, it seems that in the case of enum
classes, not using a prefix should be acceptable.

Based on Nathan's analysis (thanks, Nathan!) and the previous thread, it
seems like Emilio's edit should be reverted and a note should be added
about the usage of CamelCase for enum classes.  Emilio, do you mind doing
that, please?

Thanks,
-- 
Ehsan
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Coding style: Making the `e` prefix for enum variants not mandatory?

2018-06-29 Thread Jeff Gilbert
I don't believe e-prefix adds sufficient value.
Mutable statics *must* always be s-prefixed. If this is not the case I
will gladly bring the codebase into compliance.

On Fri, Jun 29, 2018 at 11:36 AM, smaug  wrote:
> On 06/29/2018 05:58 PM, Boris Zbarsky wrote:
>>
>> On 6/29/18 10:30 AM, Nathan Froyd wrote:
>>>
>>> Given the language-required qualification for
>>> `enum class` and a more Rust-alike syntax, I would feel comfortable
>>> with saying CamelCase `enum class` is the way to go.
>>
>>
>> For what it's worth, I agree.  The point of the "e" prefix is to highlight
>> that you have an enumeration and add some poor-man's namespacing for a
>> potentially large number of common-looking names, and the language-required
>> qualification already handles both of those.
>>
>
> That works if we're consistently naming static variables and such using
> s-prefix etc, so that
> class Foo1
> {
>   static int sBar;
> };
> Foo1::sBar
>
> is clearly different to
> enum class Foo2
> {
>   Bar
> };
>
> Foo2::Bar
>
>
> (personally I'd still prefer using e-prefix always with enums, class or not.
> Foo1::sBar vs Foo2::eBar is easier to distinguish than Foo1::sBar vs
> Foo2::Bar)
>
> -Olli
>
>
>
>
>> -Boris
>
>
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Coding style: Making the `e` prefix for enum variants not mandatory?

2018-06-29 Thread smaug

On 06/29/2018 05:58 PM, Boris Zbarsky wrote:

On 6/29/18 10:30 AM, Nathan Froyd wrote:

Given the language-required qualification for
`enum class` and a more Rust-alike syntax, I would feel comfortable
with saying CamelCase `enum class` is the way to go.


For what it's worth, I agree.  The point of the "e" prefix is to highlight that you have an enumeration and add some poor-man's namespacing for a 
potentially large number of common-looking names, and the language-required qualification already handles both of those.




That works if we're consistently naming static variables and such using 
s-prefix etc, so that
class Foo1
{
  static int sBar;
};
Foo1::sBar

is clearly different to
enum class Foo2
{
  Bar
};

Foo2::Bar


(personally I'd still prefer using e-prefix always with enums, class or not. Foo1::sBar vs Foo2::eBar is easier to distinguish than Foo1::sBar vs 
Foo2::Bar)


-Olli




-Boris


___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Coding style: Making the `e` prefix for enum variants not mandatory?

2018-06-29 Thread Jean-Yves Avenard



On 29/06/2018 16:58, Boris Zbarsky wrote:

On 6/29/18 10:30 AM, Nathan Froyd wrote:

Given the language-required qualification for
`enum class` and a more Rust-alike syntax, I would feel comfortable
with saying CamelCase `enum class` is the way to go.


For what it's worth, I agree.  The point of the "e" prefix is to 
highlight that you have an enumeration and add some poor-man's 
namespacing for a potentially large number of common-looking names, 
and the language-required qualification already handles both of those.


+1..

Would certainly be disappointed with an ALL_CAPS (being myself a user of 
kCamelCase)

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Coding style: Making the `e` prefix for enum variants not mandatory?

2018-06-29 Thread Boris Zbarsky

On 6/29/18 10:30 AM, Nathan Froyd wrote:

Given the language-required qualification for
`enum class` and a more Rust-alike syntax, I would feel comfortable
with saying CamelCase `enum class` is the way to go.


For what it's worth, I agree.  The point of the "e" prefix is to 
highlight that you have an enumeration and add some poor-man's 
namespacing for a potentially large number of common-looking names, and 
the language-required qualification already handles both of those.


-Boris
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Coding style: Making the `e` prefix for enum variants not mandatory?

2018-06-29 Thread Nathan Froyd
On Thu, Jun 28, 2018 at 7:35 PM, Emilio Cobos Álvarez  wrote:
> Oh, I didn't realize that those peers were the only ones to be able to
> update the style guide, sorry. I guess it makes sense.
>
> I can revert the change if needed and try to get sign-off from some of
> those.
>
> Apologies again, I just followed the procedure that was followed in the
> previous thread to add the rule. Let me know if you want that change
> reverted and I'll happily do so.

My sense, after grepping through the code for "enum [A-Z].* \{" (that
is, ignoring `enum class`, since those are effectively prefixed by the
language) and eyeballing the results is that ALL_CAPS and e-prefixed
enums are more common than CamelCase.  Commands:

# rough approximations only!
# enums defined on a single line
git grep -E 'enum [A-Z].* \{[^}]+\}' -- '*.[ch]*' |grep -v -e
'gfx/skia' -e 'media/webrtc' |less
# multi-line enums; -A 1 so we can see the style of the first enum
git grep -A 1 -E 'enum [A-Z].* \{' -- '*.[ch]*' |grep -v $(for p in
$(cat tools/rewriting/ThirdPartyPaths.txt); do echo -e \\x2de $p;
done) |less

This exercise was not an exhaustive analysis by any means.  (If
somebody was going to be exhaustive about this, I think it'd be
interesting to try and consider whether the enums are at global scope
or at class scope, since using class scope enums outside the class
naturally requires qualifying them with the class name.)

Based on this, I think it's reasonable to say that e-prefixing or
ALL_CAPS for (non-`enum class`) enums is the preferred style.  For
`enum class`, we (of course) do everything, but I think CamelCase is
*slightly* more common.  Given the language-required qualification for
`enum class` and a more Rust-alike syntax, I would feel comfortable
with saying CamelCase `enum class` is the way to go.

Objections?  (Almost certainly? ;)

-Nathan
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Coding style: Making the `e` prefix for enum variants not mandatory?

2018-06-28 Thread Emilio Cobos Álvarez

On 6/29/18 12:15 AM, Mike Hommey wrote:

On Thu, Jun 28, 2018 at 05:27:23PM +0200, Emilio Cobos Álvarez wrote:

I asked kats@ (which added the list item regarding enums) and he was fine
removing it from the coding style, so given that and the rest of the thread
I've edited the page accordingly.


Not that I'd oppose, but I'll note that neither kats, nor participants to
this thread so far are peers of the "C++/Rust usage, tools, and style"
module.


Oh, I didn't realize that those peers were the only ones to be able to 
update the style guide, sorry. I guess it makes sense.


I can revert the change if needed and try to get sign-off from some of 
those.


Apologies again, I just followed the procedure that was followed in the 
previous thread to add the rule. Let me know if you want that change 
reverted and I'll happily do so.


 -- Emilio



Mike


___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Coding style: Making the `e` prefix for enum variants not mandatory?

2018-06-28 Thread Mike Hommey
On Thu, Jun 28, 2018 at 05:27:23PM +0200, Emilio Cobos Álvarez wrote:
> I asked kats@ (which added the list item regarding enums) and he was fine
> removing it from the coding style, so given that and the rest of the thread
> I've edited the page accordingly.

Not that I'd oppose, but I'll note that neither kats, nor participants to
this thread so far are peers of the "C++/Rust usage, tools, and style"
module.

Mike
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Coding style: Making the `e` prefix for enum variants not mandatory?

2018-06-28 Thread Emilio Cobos Álvarez
I asked kats@ (which added the list item regarding enums) and he was 
fine removing it from the coding style, so given that and the rest of 
the thread I've edited the page accordingly.


Cheers,

 -- Emilio

On 6/26/18 2:46 AM, Jeff Gilbert wrote:

If we can't agree, it shouldn't be in the style guide.

On Mon, Jun 25, 2018 at 2:17 PM, Peter Saint-Andre  wrote:

And perhaps good reason for removing it from the style guide? ;-)

On 6/25/18 3:08 PM, Emilio Cobos Álvarez wrote:

And Kris pointed out that we already had another huge thread on this:


https://groups.google.com/d/msg/mozilla.dev.platform/WAuySoTfq_w/-DggRotpBQAJ


Looks like there wasn't agreement on that one... But oh well, don't want
to repeat a lot of that discussion.

I think the argument for consistency with the other systems language we
have in-tree, the fact that it's not predominant (at least for enum
classes) even though it is in the coding style, and that there wasn't
agreement in the previous thread are good reasons for not enforcing it,
but...

  -- Emilio

On 6/25/18 10:41 PM, Emilio Cobos Álvarez wrote:

Our coding style states that we should use an `e` prefix for enum
variants, that is:

enum class Foo { eBar, eBaz };

We're not really consistent about it: looking at layout/, we mostly
use CamelCase, though we do have some prefixed enums. Looking at other
modules, enum classes almost never use it either. DOM bindings also
don't use that prefix.

I think that with enum classes the usefulness of the prefix is less
justified. Plus removing them would allow us to match the Rust coding
style as well, which is nice IMO.

Would anybody object to making the prefix non-mandatory, removing that
line from the coding style doc? Maybe only making it non-mandatory for
enum classes?

Thanks,

   -- Emilio
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Coding style: Making the `e` prefix for enum variants not mandatory?

2018-06-25 Thread Jeff Gilbert
If we can't agree, it shouldn't be in the style guide.

On Mon, Jun 25, 2018 at 2:17 PM, Peter Saint-Andre  wrote:
> And perhaps good reason for removing it from the style guide? ;-)
>
> On 6/25/18 3:08 PM, Emilio Cobos Álvarez wrote:
>> And Kris pointed out that we already had another huge thread on this:
>>
>>
>> https://groups.google.com/d/msg/mozilla.dev.platform/WAuySoTfq_w/-DggRotpBQAJ
>>
>>
>> Looks like there wasn't agreement on that one... But oh well, don't want
>> to repeat a lot of that discussion.
>>
>> I think the argument for consistency with the other systems language we
>> have in-tree, the fact that it's not predominant (at least for enum
>> classes) even though it is in the coding style, and that there wasn't
>> agreement in the previous thread are good reasons for not enforcing it,
>> but...
>>
>>  -- Emilio
>>
>> On 6/25/18 10:41 PM, Emilio Cobos Álvarez wrote:
>>> Our coding style states that we should use an `e` prefix for enum
>>> variants, that is:
>>>
>>>enum class Foo { eBar, eBaz };
>>>
>>> We're not really consistent about it: looking at layout/, we mostly
>>> use CamelCase, though we do have some prefixed enums. Looking at other
>>> modules, enum classes almost never use it either. DOM bindings also
>>> don't use that prefix.
>>>
>>> I think that with enum classes the usefulness of the prefix is less
>>> justified. Plus removing them would allow us to match the Rust coding
>>> style as well, which is nice IMO.
>>>
>>> Would anybody object to making the prefix non-mandatory, removing that
>>> line from the coding style doc? Maybe only making it non-mandatory for
>>> enum classes?
>>>
>>> Thanks,
>>>
>>>   -- Emilio
>>> ___
>>> dev-platform mailing list
>>> dev-platform@lists.mozilla.org
>>> https://lists.mozilla.org/listinfo/dev-platform
>> ___
>> dev-platform mailing list
>> dev-platform@lists.mozilla.org
>> https://lists.mozilla.org/listinfo/dev-platform
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Coding style: Making the `e` prefix for enum variants not mandatory?

2018-06-25 Thread Peter Saint-Andre
And perhaps good reason for removing it from the style guide? ;-)

On 6/25/18 3:08 PM, Emilio Cobos Álvarez wrote:
> And Kris pointed out that we already had another huge thread on this:
> 
> 
> https://groups.google.com/d/msg/mozilla.dev.platform/WAuySoTfq_w/-DggRotpBQAJ
> 
> 
> Looks like there wasn't agreement on that one... But oh well, don't want
> to repeat a lot of that discussion.
> 
> I think the argument for consistency with the other systems language we
> have in-tree, the fact that it's not predominant (at least for enum
> classes) even though it is in the coding style, and that there wasn't
> agreement in the previous thread are good reasons for not enforcing it,
> but...
> 
>  -- Emilio
> 
> On 6/25/18 10:41 PM, Emilio Cobos Álvarez wrote:
>> Our coding style states that we should use an `e` prefix for enum
>> variants, that is:
>>
>>    enum class Foo { eBar, eBaz };
>>
>> We're not really consistent about it: looking at layout/, we mostly
>> use CamelCase, though we do have some prefixed enums. Looking at other
>> modules, enum classes almost never use it either. DOM bindings also
>> don't use that prefix.
>>
>> I think that with enum classes the usefulness of the prefix is less
>> justified. Plus removing them would allow us to match the Rust coding
>> style as well, which is nice IMO.
>>
>> Would anybody object to making the prefix non-mandatory, removing that
>> line from the coding style doc? Maybe only making it non-mandatory for
>> enum classes?
>>
>> Thanks,
>>
>>   -- Emilio
>> ___
>> dev-platform mailing list
>> dev-platform@lists.mozilla.org
>> https://lists.mozilla.org/listinfo/dev-platform
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Coding style: Making the `e` prefix for enum variants not mandatory?

2018-06-25 Thread Emilio Cobos Álvarez

And Kris pointed out that we already had another huge thread on this:


https://groups.google.com/d/msg/mozilla.dev.platform/WAuySoTfq_w/-DggRotpBQAJ

Looks like there wasn't agreement on that one... But oh well, don't want 
to repeat a lot of that discussion.


I think the argument for consistency with the other systems language we 
have in-tree, the fact that it's not predominant (at least for enum 
classes) even though it is in the coding style, and that there wasn't 
agreement in the previous thread are good reasons for not enforcing it, 
but...


 -- Emilio

On 6/25/18 10:41 PM, Emilio Cobos Álvarez wrote:
Our coding style states that we should use an `e` prefix for enum 
variants, that is:


   enum class Foo { eBar, eBaz };

We're not really consistent about it: looking at layout/, we mostly use 
CamelCase, though we do have some prefixed enums. Looking at other 
modules, enum classes almost never use it either. DOM bindings also 
don't use that prefix.


I think that with enum classes the usefulness of the prefix is less 
justified. Plus removing them would allow us to match the Rust coding 
style as well, which is nice IMO.


Would anybody object to making the prefix non-mandatory, removing that 
line from the coding style doc? Maybe only making it non-mandatory for 
enum classes?


Thanks,

  -- Emilio
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Coding style: Making the `e` prefix for enum variants not mandatory?

2018-06-25 Thread Emilio Cobos Álvarez
Our coding style states that we should use an `e` prefix for enum 
variants, that is:


  enum class Foo { eBar, eBaz };

We're not really consistent about it: looking at layout/, we mostly use 
CamelCase, though we do have some prefixed enums. Looking at other 
modules, enum classes almost never use it either. DOM bindings also 
don't use that prefix.


I think that with enum classes the usefulness of the prefix is less 
justified. Plus removing them would allow us to match the Rust coding 
style as well, which is nice IMO.


Would anybody object to making the prefix non-mandatory, removing that 
line from the coding style doc? Maybe only making it non-mandatory for 
enum classes?


Thanks,

 -- Emilio
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform