-
Notifications
You must be signed in to change notification settings - Fork 229
Remove enum prefixes #187
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove enum prefixes #187
Changes from all commits
0d6ced5
7746b91
53a7df0
9da923d
17e31f4
a53d805
b3b7c00
4a4429d
48e80cf
86d7c30
5c8e926
f10bec4
e04fcb6
5141d41
b9a50a7
0869e69
fe3addf
b82517d
9fd8917
264c4b9
d965741
9b9f737
60d1880
ae80862
5adc93d
a7df010
d58ce63
17ba889
b1ba675
e2b850e
bbe7256
ecc1629
7b6f03d
6592d4d
6fe9b5c
3afffca
52456c7
ce25f96
992db51
72a8b89
a2ba23e
25ef492
383575f
ecae7f6
0cc1790
6392a9d
a70a00d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -11,3 +11,11 @@ def pythonize_field_name(name: str) -> str: | |||||||||||
|
||||||||||||
def pythonize_method_name(name: str) -> str: | ||||||||||||
return casing.safe_snake_case(name) | ||||||||||||
|
||||||||||||
|
||||||||||||
def pythonize_enum_member_name(name: str, enum_name: str) -> str: | ||||||||||||
enum_name = casing.snake_case(enum_name).upper() | ||||||||||||
find = name.find(enum_name) | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think removing the enum name from just anywhere in the key name would be a good idea, especially if each key is evaluated independently.
Suggested change
I wonder if it's better to be all or nothing here... i.e. only do the renaming if the convention is perfectly followed (by all keys). Do you know of a precedent or documented rule to follow one way or the other? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I used find because sometimes people in C use k_ to prefix an enum. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why would a prefix of Do we just need a small test for this case (and any others)? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because the enum name doesn't start with k_ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I still think this implementation is unclear and error prone. For example imagine the chaos given an enum type with a single letter name! If the intention is to remove a "prefix", then just remove the prefix. If you feel it's necessary to make it more clever than that (which I'm skeptical of) then a docstring explanation and test coverage are essential. |
||||||||||||
if find != -1: | ||||||||||||
name = name[find + len(enum_name) :].strip("_") | ||||||||||||
return casing.sanitize_name(name) |
Uh oh!
There was an error while loading. Please reload this page.