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

Stabilize generated MethodDescriptor API #1901

Closed
ejona86 opened this issue Jun 7, 2016 · 4 comments
Closed

Stabilize generated MethodDescriptor API #1901

ejona86 opened this issue Jun 7, 2016 · 4 comments
Labels
enhancement experimental API Issue tracks stabilizing an experimental API performance
Milestone

Comments

@ejona86
Copy link
Member

ejona86 commented Jun 7, 2016

Today, the generated method descriptors are available as fields. This should probably be changed to be behind a method of some sort so that they can be generated lazily (to reduce static class loading time for Android).

@ejona86 ejona86 added this to the 1.1 milestone Jun 7, 2016
@ejona86 ejona86 modified the milestones: Next, 1.1 Aug 30, 2016
@zpencer zpencer added the experimental API Issue tracks stabilizing an experimental API label Sep 19, 2017
@zpencer
Copy link
Contributor

zpencer commented Sep 19, 2017

Seems like an API related issue, adding tag for easier searching

@ejona86
Copy link
Member Author

ejona86 commented Oct 31, 2017

#3618 adds methods for the fields. After giving a little time to convert code over, the fields can be removed.

@ejona86
Copy link
Member Author

ejona86 commented May 31, 2018

API review notes:

  • The getFooMethod() methods are fine to stabilize.
  • There is some concern about collisions due to CamelCasing vs snake_casing. That may need to be resolved first.

@ejona86
Copy link
Member Author

ejona86 commented Jun 1, 2018

The current casing behavior seems fine.

The snake_casing collision concern is no stronger for this new method than for our existing stub methods. We assume CamelCasing for service names which matches protobuf's generic services. Accepting snake_casing methods and converting them also matches protobuf's generic services. While it is true it can cause collisions if a user uses snake_casing, I think our answer will be "don't do that." Especially since the style guide says to use CamelCase.

Because of the style guide, we could conceivably ignore the snake_casing conversion and would still produce the same results as we do today for style-conforming code and avoid collisions for non-conforming code. However, it's not worth the risk to change our pre-existing methods, and there's no gain to change this new API to be different from our current stub methods.

@ejona86 ejona86 modified the milestones: Next, 1.13 Jun 11, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Sep 28, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement experimental API Issue tracks stabilizing an experimental API performance
Projects
None yet
Development

No branches or pull requests

2 participants