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 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: Rust crate approval

2018-06-29 Thread Tom Ritter
On Thu, Jun 28, 2018 at 11:42 PM, Nathan Froyd  wrote:

> We have generally trusted people to use good judgement in what they
> use and how much review is required.  Accordingly, I think you should
> request review from the people who would normally review your code,
> and if you have concerns about specific crates that are being
> vendored, you should call those crates out as needing especial review.
> If you or your reviewers think such reviews fall outside of your
> comfort zone/area of expertise/Rust capabilities, please flag myself
> or Ehsan, and we will work on finding people to help.
>

I know that enumerating badness is never a comprehensive solution; but
maybe there could be a wiki page we could point people to for things that
indicate something is doing something scary in Rust?  This might let us
crowd-source these reviews in a safer manner. For example, what would I
look for in a crate to see if it was:
 - Adjusting memory permissions
 - Reading/writing to disk
 - Performing unsafe C/C++ pointer stuff
 - Performing network connections of any type
 - Calling out to syscalls or other kernel functions (especially win32k.sys
functions on Windows)
 - (whatever else you can think of...)

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