Migrate to protobuf-lite #20
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
edaniels
requested review from
njooma and
stuqdog
and removed request for
a team
edaniels
force-pushed
the
protobuf-lite
branch
2 times, most recently
from
74d73e3 to
897a8d1
Compare
njooma
approved these changes
Apr 11, 2024
Yeah my vote would be just duplicate the code. There's no commonality between lite and java. They way you get custom APIs is to have user include the proto FileDescriptorSets which buf can easily create and put in a resources directory for you. protobuf-java does it automatically, unlike -lite |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This makes custom modular APIs not possible until we add some extra code but this will increase compatibility a ton. Yes we are just trading to another evil/compromise (protocolbuffers/protobuf#8104), but I believe in cases where someone is new to Viam and bringing their own module API, they are likely able to use protobuf-lite. If someone is on the server and they truly need protobuf-java, we should consider abstracting the SDK a bit more across the platforms to have them provide separate protobuf-java and protobuf-lite generated files and more generic grpc client/service classes to support this. As of right now based on how this SDK is constructed, the two cannot co-exist and we've seen more android use cases anyway.