feat(jsii): enforce enum names to be UPPER_CASE#541
Conversation
eladb
left a comment
eladb
left a comment
There was a problem hiding this comment.
No..... enum member names, not type names :-(
|
|
||
| function _typeNamesMustUsePascalCase(_: Validator, assembly: spec.Assembly, diagnostic: DiagnosticEmitter) { | ||
| for (const type of _allTypes(assembly)) { | ||
| if (spec.isEnumType(type)) { continue; } |
There was a problem hiding this comment.
The enum /type/ should be pascal case, just members should be capital
There was a problem hiding this comment.
ah i see, totally misread that - so enum values then?
i'll get on it.
There was a problem hiding this comment.
Enum members:
export enum MyEnum {
MEMBER_NUMBER_ONE,
MEMBER_FOO,
MEMBER_XOO
}| @@ -1,4 +1,4 @@ | |||
| ///!MATCH_ERROR: Type names must use PascalCase: My_Enum | |||
| ///!MATCH_ERROR: Enum names must use TRUMP_CASE: My_Enum | |||
There was a problem hiding this comment.
Nope! Just the member names should be TRUMP_CASE
There was a problem hiding this comment.
I think my Java disposition's are showing - enum names vs enum values. I'll get it updated!
eladb
left a comment
eladb
left a comment
There was a problem hiding this comment.
Need to make a decision about python and .NET. Not sure what's the idiomatic way to express enum members in those languages.
| Assert.Equal(EnumFromScopedModule.Value1, obj.LoadFoo()); | ||
| obj.SaveFoo(EnumFromScopedModule.Value2); | ||
| Assert.Equal(EnumFromScopedModule.Value2, obj.Foo); | ||
| Assert.Equal(EnumFromScopedModule.VALUE2, obj.Foo); |
There was a problem hiding this comment.
wait... what's the convention for .NET? Do we want .NET to also use all-caps?
Same question for Python.... @garnaat is it idiomatic to use all caps for enum values?
There was a problem hiding this comment.
- .NET seems to prefer PascalCase.
- Python looks like ALL_CAPS are good.
So basically, the .NET code generator needs to covert the case for .NET.
There was a problem hiding this comment.
that's a good question. I thought we wanted to avoid dropping into the language specific mangling... let me check.
There was a problem hiding this comment.
I think Pascal casing for both names and values are the idiomatic thing for .NET
There was a problem hiding this comment.
No. We wanted to normalize the typescript/jsii source to CAPITALS but then, jsii is all about language-specific idiomacy.
There was a problem hiding this comment.
Sounds good. I'll make the changes to the generation of .NET so we can change enum values into PascalCase.
There was a problem hiding this comment.
we decided we would make the changes for .NET generation to use enum members in PascalCase outside of this PR.
Should I create an issue for it before resolving this conversation?
There was a problem hiding this comment.
Sure, "right click, create issue"
2 participants
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.