-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Grpc #3733
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
pongad
left a comment
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.
Wait sorry. I think we need to update GAX's gRPC as well right...?
|
Possibly. This is an experiment to see if it breaks. Not quite ready to commit to this yet. One thing I'm curious about is why we're setting these versions in multiple places. |
|
@elharo I think it's because the |
|
Yes, that seems right. Should they share a parent? or perhaps each import a BOM that manages the dependencies for both? |
|
Theoretically we shouldn't need to upgrade GAX at the same time since it's on 1.13.1 and 1.15.0 (minor version bump) is supposed to be backwards compatible with that. But I wouldn't bet heavily on it. |
|
@elharo, @pongad So the current separation is by design. It is annoying to have to repeat same dependency multiple times, but unfortunately it is not the only place (there are many dependencies duplications, not only in this repo), so fixing it here will not fix the actual problem. How to fix that dependency duplication problem is actually a very complicated problem (out of scope here). (one of the first googled stackoverflow links regarding parent-child groupId relation) |
|
@vam-google Do you have a reference for not putting different group IDs under the same parent? Happy not to do it here, but it may be relevant for some PRs in other repos I'm looking at. |
|
When I was doing it I remember that putting those under same parent didn't really work (I'm not saying it did not compile, but it caused more problems than solved). As for the reference, the one, with pretty weak wording (describing it only as "A good way", not a "must") is in the maven documentation: "A good way to determine the granularity of the groupId is to use the project structure. That is, if the current project is a multiple module project, it should append a new identifier to the parent's groupId. For example, Notice, that children must share prefix, the only way for our case to share prefix is to have |
|
Looks like the CI hung. Anyone happen to know how to force it to restart? |
|
@chingor13 how do we trigger Kokoro to run? |
|
We add the |
chingor13
left a comment
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, @garrettjonesgoogle can merge when gax is updated as well.
|
It's fine to merge this before GAX |
It's not immediately obvious why these two pom.xml files separately declare versions.