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

fix: handle empty protobuf package #274

Closed
wants to merge 3 commits into from
Closed

fix: handle empty protobuf package #274

wants to merge 3 commits into from

Conversation

kilimnik
Copy link
Contributor

See #273, the generators don't work correctly on empty package names. I fixed that by checking if the package name is empty and using it when needed.

@stephenh
Copy link
Owner

@dk99 thanks for the PR!

Can you add a new integration-test for this, i.e. make a new directory in integration/ with a name like no-proto-package, and put a ~as-minimal-as-possible proto file that reproduces your issue?

Then include the generated output in the PR, to both review + function as a regression test.

Thanks!

@stephenh stephenh changed the title handle empty protobuf package fix: handle empty protobuf package Apr 16, 2021
@kilimnik
Copy link
Contributor Author

I have added one minimal integration test. I am unsure if that test covers all possibilities. To cover all possibilities it would be necessary to also compile this test for grpc-web.

@stephenh
Copy link
Owner

Thanks @dk99 ! I wanted to make a small tweak and haven't learned how to push to existing PRs, so I created #285 and will release from there. Thanks!

@stephenh stephenh closed this Apr 24, 2021
zfy0701 added a commit to sentioxyz/ts-proto that referenced this pull request Jan 5, 2023
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.

2 participants