I'm not sure how this is compromise. We were already supposed to use
enum classes with new code.
Every additional glyph incurs mental load. Code should instead be
written so these things are not necessary to explicitly state. *That*
is the code we should be trying to write. In well-written code,
superfluous warts detract from readability.
At this point, it feels a lot like I could propose full hungarian
notation to this list piecemeal, and there would be ongoing
'compromises' to add it one step at a time. After all, who wants to
trace back to the variable declaration when the variable name can tell
us what type it is?
I propose that if you're in a part of the codebase where you can't
tell if it's an enum or a function pointer (regardless of the fact
that the compiler can't confuse these), you should do more looking
around before working on it, much less reviewing changes to the code.
If you can't tell if it's an enum, you're not going to be able to
effectively make changes, so buckle up and do some looking around.
To go completely off the rails, I no longer even believe that all of
Gecko should have a single style. I think the whole attempt is
increasingly a distraction vs the alternative, and I say this as
someone who writes and reviews under at least three differently-styled
code areas. The JS engine will, as ever, remain distinct, as will
third-party code, and also all non-actively maintained files. (most of
our codebase)
Yes, a formatter could magic-wand us to most of the superficial
aspects of a single code style, but we'd then need to backfill
readability into some of the trickier bits. Sometimes we need to step
outside the prevailing style to keep something reasonably readable.
But we /already have/ a process for keeping things readable: Code reviews.
And if that fails: Module ownership.
I'm not sure why we hold centralization in such high regard when it
comes to code style. And, if we are continuing to centralize this,
we're going to need to have an actual process for making decisions
here.
I suspect the goal is to have an oracle we can ask how we should
format code in order to be readable. However, we can't even agree on
it, so the right answer often is indeterminate. However, people within
their modules have a better idea of what works in each case.
I do think we should have some high-level style guidance, but I think
we're increasingly micromanaging style. Let code review take care of
the specifics here. If it's not readable without prefixes, definitely
consider adding prefixes. Otherwise don't require it.
FWIW, I do have a preference for k prefix instead of e prefix, (if we
must have a required prefix) since they are in the same 'constants'
category. 'k' I can be persuaded the usefulness of for enums. 'e', not
so much.
On Mon, Apr 11, 2016 at 8:00 AM, Kartikaya Gupta <kgu...@mozilla.com> wrote:
> Based on all the feedback so far I think the best compromise is to use
> enum classes with the "e" prefix on the values. As was mentioned, the
> "e" prefix is useful to distinguish between enum values and function
> pointer passing at call sites, but is a small enough prefix that it
> shouldn't be too much of a burden.
>
> I realize not everybody is going to be happy with this but I don't
> want this discussion to drag on to the point where it's a net loss of
> productivity no matter what we end up doing. I'll update the style
> guide.
>
> Thanks all!
>
> kats
>
>
> On Sun, Apr 10, 2016 at 11:43 AM, smaug <sm...@welho.com> wrote:
>> On 04/10/2016 05:50 PM, Aryeh Gregor wrote:
>>>
>>> On Fri, Apr 8, 2016 at 8:35 PM, smaug <sm...@welho.com> wrote:
>>>>
>>>> enum classes are great, but I'd still use prefix for the values.
>>>> Otherwise the values look like types - nested classes or such.
>>>> (Keeping my reviewer's hat on, so thinking about readability here.)
>>>
>>>
>>> In some cases I think it's extremely clear, like this:
>>>
>>> enum class Change { minus, plus };
>>>
>>> whose sole use is as a function parameter (to avoid a boolean
>>> parameter), like so:
>>>
>>> ChangeIndentation(*curNode->AsElement(), Change::plus);
>>>
>>> In cases where it might be mistaken for a class, it might make more
>>> sense to prefix the enum class name with "E". I don't think this
>>> should be required as a general policy, though, because for some enums
>>> it might be clear enough without it.
>>>
>>
>> Indeed in your case it is totally unclear whether one is passing a function
>> pointer or enum value as param.
>> If the value was prefixed with e, it would be clear.
>>
>> Consistency helps with reviewing (and in general code readability) a lot, so
>> the less we have optional coding style rules, or
>> context dependent rules, the better.
>>
>>
>>
>>
>>
>> _______________________________________________
>> 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
