-
Notifications
You must be signed in to change notification settings - Fork 421
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
Stop requiring 'Info' at the end of all CamelCase names. #1163
base: main
Are you sure you want to change the base?
Conversation
With this PR, you're allowing any variable name to use either |
@brandjon could you please review it? I haven't been working on Starlark for a long time and don't know what naming is actually recommended now. |
I'll defer to @brandjon here; he has much more context, my personal opinion is that this loos like a good idea, but I don't have all the information swapped into my brain to fully evaluate. |
I definitely wouldn't encourage UpperCamelCase outside of provider types. A quick search within Google's monorepo yields that well under 10% of providers forego the "Info" suffix. If my memory's correct, I proposed this naming convention. If I had to do it over again I might not choose it. But I wouldn't step away from it at this point for no reason. There's several other dubious names that are now idiosyncratic in Bazel; "depset" (another one of mine) and "provider" are the glaring ones that come to mind. I'd be hesitant of change for change's sake unless we're really confident the new way is much better. I haven't heard a case for that yet. We would like to do something with types in Starlark/Bazel. It's possible this conversation could arise again then. |
It doesn't add any new information. It's like saying "TransitiveProtoSourcesClass" instead of "TransitiveProtoSources".