Skip to content
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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

aiuto
Copy link
Contributor

@aiuto aiuto commented Apr 27, 2023

It doesn't add any new information. It's like saying "TransitiveProtoSourcesClass" instead of "TransitiveProtoSources".

@laurentlb
Copy link
Contributor

With this PR, you're allowing any variable name to use either lower_snake_case or UpperCamelCase. I don't think we want to allow UpperCamelCase everywhere.

@vladmos
Copy link
Member

vladmos commented Apr 27, 2023

@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.

@aiuto aiuto requested a review from lberki June 14, 2023 15:30
@lberki
Copy link

lberki commented Jun 15, 2023

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.

@lberki lberki removed their request for review June 15, 2023 07:38
@brandjon
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants