-
Notifications
You must be signed in to change notification settings - Fork 843
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
Update proto to latest proto commit #2027
Conversation
Signed-off-by: Bogdan Drutu <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #2027 +/- ##
============================================
+ Coverage 84.37% 84.39% +0.01%
- Complexity 1879 1880 +1
============================================
Files 223 223
Lines 7585 7585
Branches 808 808
============================================
+ Hits 6400 6401 +1
Misses 874 874
+ Partials 311 310 -1
Continue to review full report at Codecov.
|
Wait! Is this a released version of the proto? |
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.
We should not use a non-released version of the proto
@jkwatson do we? |
I assert yes, we should only use released versions of the protobuf. |
Can we please then do a release of proto and then another one for the java package? |
Can you explain why this is needed so urgently? We are on a 2 week release cycle now, and do not generally intend to release patch releases unless there is an urgent bugfix. |
Because we need the new |
That explains what you need, but not why it is so urgent that we would be forced to do a patch release. |
Also, just updating the protobuf won't mean that we're actually using the protobuf structures for anything. How is just updating the protobuf definitions going to help you at all? |
Because we are using opentelemetry-proto as transport layer for our use case. |
So...let me understand. You're using the java repo's published generated protobuf classes for something other than use in the SDK? That does not sound like a reason for us to do an emergency patch release. If you need these protobuf classes for something outside the use in the SDK, I recommend setting up a project that will build and publish those for you, rather than depending on this project which just happens to publish them as a part of the release process. |
IMO, the correct solution to this problem is to have the protobuf repository publish various language bindings. Then, the opentelemetry-java project could depend on them like a normal library (rather than as a git submodule), and usages by other projects would be supported as well. |
I agree that publishing stubs from the opentelemetry-proto repo would be ideal for contributor experience and version management. I have sent open-telemetry/opentelemetry-proto#231 as a proposal to start to make that happen. For reference, I've done something similar on a previous project and it works pretty well https://github.com/infostellarinc/stellarstation-api |
Would it be possible to create a new release of the proto with |
Please post that question in the proto repo, since it's not something the java folks control. |
closing this. when the proto project cuts a release, I'll be happy to update our dependency. |
Related to #2021
Signed-off-by: Bogdan Drutu [email protected]