fix(build_gen): use consistent names for open source package #59
Conversation
google-cla
bot
added
the
cla: yes
|
Goals:
the alias should minimize these effects. if we remove the deprecation message there should be no blast radius |
|
CC @vam-google Thanks for the context, @jpoehnelt. Since the objective of this change is primarily for a non-cloud use case, could we instead introduce a We would like to support non-Cloud APIs as well, and these seem like different use cases. As the vast majority of our users consist of Cloud APIs, introducing a different run-mode could help address your (Geo's?) use case while maintaining existing behavior for Cloud teams. |
|
I'm not sure the complexity of adding a different run mode is worth it. This change should have zero impact on cloud teams. |
|
I guess it would change the tar output name. :/ |
There was a problem hiding this comment.
That naming is horrible and comes from the package names produced by Artman (legacy tool used to generate these things). Unfortunately too many things are currently depending on the generated name format for each individual language (like owl-bot used to populate googleapis-gen repository, many internal tools/scripts which we are not even aware of).
In other words, I agree that the current naming is very bad, but unless somebody has capacity to fix not only current naming but everything else which already depends on this naming, I believe we have to keep it as is.
There was a problem hiding this comment.
Please do not do this. This adds too many hacks to the generated file to fix a relatively minor issue with naming. Notice, this file is on the surface of all builds. People interact with it very often, it must be kept as clean as possible. Local variables and aliasing is not idiomatic for build files and in general over complicates things without providing much benefit.
Please think about it this way: if we push this change, it will sacrifice the readability of build files of all cloud in the name of Google Maps. With all respect to maps, we can't do it.
If you really need a custom template, you can specify it as a command line arg to the build file generator: https://github.com/googleapis/rules_gapic/blob/main/bazel/src/main/java/com/google/api/codegen/bazel/ArgsParser.java#L75
| @@ -81,14 +81,22 @@ java_gapic_test( | |||
| ) | |||
|
|
|||
| # Open Source Packages | |||
| java_gapic_assembly_gradle_pkg_deps = [ | |||
There was a problem hiding this comment.
this is too verbose and non-idiomatic (to keep dependencies in a separate variable; also in that chase the variable shoudd be named _LIKE_THIS).
| @@ -97,6 +97,12 @@ java_gapic_assembly_gradle_pkg( | |||
| ], | |||
| ) | |||
|
|
|||
| alias( | |||
There was a problem hiding this comment.
This adds aliasing to solve the non-pretty name issue. This is too verbose and seems not worth it.
This makes it easier to depend on these rules. I added a deprecated alias to maintain the current names.
closes #56