-
Notifications
You must be signed in to change notification settings - Fork 14
feat!: migrate to use microgen #23
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
Conversation
I noticed there are now 3 modules: |
Yes, |
Isn't that going to be a bad developer experience for the user? That we expose both |
For all client libraries, we have one folder for each version, for instance, |
I see, so |
yes
As far as I understand, users don't use the auto generated layer at all so this shouldn't be a problem. |
Echoing what @simonz130 said earlier in a chat thread, error reporting is an API that is probably too convoluted to use without the wrapper. See this bit of code for example that unrolls a traceback into the format the APi expects. We could remove the unversioned alias if you think it's likely to cause confusion. |
I can remove the alias if you guys would like so. |
That would be my vote, but I'll defer to @simonz130 if he has other opinions I still wonder if there would be a better way to handle hand-written vs generated packages though. Even if it's not an issue for this one, it may come up with other libraries. (It took me a while to realize that logging and logging_v2 weren't two versions of the logging library, but two separate libraries with different use cases) But I can save that discussion for later! |
I just updated the PR to remove the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
(1) Migrate the auto-generated errorreporting_v1beta1 to use microgenerator.
(2) There is no interface changes to the handwritten layer, so users won't be affected.
(3) bump the major version because this PR drops python 2.7 support.