fix(Spring CodeGen): Update and move naming-related logic into utils #1061

emmileaf · 2022-10-13T17:51:42Z

This PR updates and moves three pieces of naming-related logic in spring codegen into util methods:

getSpringPropertyPrefix (e.g. com.google.cloud.vision.v1.spring.auto.image-annotator)
- Replaced earlier use of libname due to ambiguity (see comment)
getLibName (e.g. vision)
- Intended for descriptive purposes
- Includes three options considered by design doc - currently this PR implements option 2 (parse from package name) with the intention to switch to option 3 (parse from default host) in alignment with SampleGen
getSpringPackageName (e.g. com.google.cloud.vision.v1.spring)
- Since this was also used in a couple of places, extracted to util in case of future need to update

Dependencies:

feat(Spring CodeGen): Add customized spring composer for package info #1059 should be adjusted to use these util methods (merge order should be after this PR)

… both to utils

conventional-commit-lint-gcf · 2022-10-13T17:51:47Z

🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use automerge label. Good luck human!

-- conventional-commit-lint bot
https://conventionalcommits.org/

conventional-commit-lint-gcf · 2022-10-13T17:51:47Z

🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use automerge label. Good luck human!

-- conventional-commit-lint bot
https://conventionalcommits.org/

emmileaf · 2022-10-13T19:44:17Z

Two questions for perhaps an initial review related to the code changes here:

Would we prefer to have this PR wait on the switch to parseDefaultHost in getLibName, or make the change later in a separate PR?
I saw that the work in feat(Spring CodeGen): write pom.xml with SpringWriter. #1057 also needs things like the client library name (descriptive title vs. short name), group, and version. Would it make sense for these parsing logic (or at least the tentative version) to also be added under Utils?

In parsing version, for example, perhaps we could also align with SampleGen’s approach:https://github.com/googleapis/gapic-generator-java/blob/d222af10cd6345e0a3660353d7d667e917be67ec/src/main/java/com/google/api/generator/gapic/composer/Composer.java#L194-L205

zhumin8 · 2022-10-17T14:54:47Z

I think it makes sense to make the change later in a separate PR, consider parseDefaultHost exposure may need to wait a bit.
As for feat(Spring CodeGen): write pom.xml with SpringWriter. #1057, I would suggest you not to worry about it in this pr. I will align later in that pr separately when possible. The client library coordinates likely needs to be acquired outside of gapic-gen from sources like this metadata, and the library name in Spring Starter to produced can align with parseDefaultHost in getLibName.

emmileaf · 2022-10-17T15:29:58Z

@zhumin8 Sounds good! In that case I’ll keep both items as out of scope for this PR and mark it ready for review.

(Side note for Sonar: the missing code coverage for these utils should improve after #1059 adds package info composer tests)

zhumin8

Just minor comments.

zhumin8 · 2022-10-17T19:06:28Z

src/main/java/com/google/api/generator/spring/SpringWriter.java

-                          Utils.springPropertyPrefix(libName, service.name()),
-                          libName + "/" + service.name())));
+                          Utils.getSpringPropertyPrefix(packageName, service.name()),
+                          Utils.getLibName(context))));


I think we still want service.name() some where in the description, as this block is generated per service. So for client lib with multiple services like vision, description for enabling each service is different.

Oops - thanks for catching this!

zhumin8 · 2022-10-17T19:13:46Z

src/main/java/com/google/api/generator/spring/SpringWriter.java

    JarEntry jarEntry =
        new JarEntry(String.format("%s/additional-spring-configuration-metadata.json", path));
-    String libName = Utils.getLibName(context);
+    String packageName = context.services().get(0).pakkage();


nit: Do you mind also extract this to Utils? It's only one line, but just in case of future changes.

…e to utils

sonarqubecloud · 2022-10-18T13:14:28Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
2 Code Smells

58.3% Coverage
0.0% Duplication

This is a draft to generate pom.xml for Spring Starters modules. The placeholders in this generated file will be replaced in spring-cloud-gcp when triggering the generation (e.g. via script) Update: updated to use utils added in #1061 We'll likely need to make more changes to pom generation depending on the project structure on spring-cloud-gcp side. But I'd like to merge this in first, and iterate as needed together with spring-cloud-gcp side.

Remove use of libname in spring properties prefix, and move logic for…

dee10ce

… both to utils

emmileaf added the spring pr that's related to spring code gen, intend to merge into autoconfig-gen-draft2 branch. label Oct 13, 2022

Update expected output in comment

83cf256

emmileaf mentioned this pull request Oct 13, 2022

feat(Spring CodeGen): Add customized spring composer for package info #1059

Merged

2 tasks

emmileaf requested review from diegomarquezp and zhumin8 October 13, 2022 19:43

emmileaf marked this pull request as ready for review October 17, 2022 15:30

emmileaf requested a review from a team October 17, 2022 15:30

emmileaf requested a review from a team as a code owner October 17, 2022 15:30

zhumin8 reviewed Oct 17, 2022

View reviewed changes

Add service name in per-service description and extract getPackageNam…

c97f59e

…e to utils

emmileaf requested a review from zhumin8 October 18, 2022 13:46

zhumin8 approved these changes Oct 18, 2022

View reviewed changes

emmileaf merged commit 07dfa79 into autoconfig-gen-draft2 Oct 18, 2022

emmileaf deleted the autoconfig-naming branch October 18, 2022 14:37

zhumin8 mentioned this pull request Oct 21, 2022

feat(Spring CodeGen): write pom.xml with SpringWriter. #1057

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(Spring CodeGen): Update and move naming-related logic into utils #1061

fix(Spring CodeGen): Update and move naming-related logic into utils #1061

Uh oh!

emmileaf commented Oct 13, 2022 •

edited

Loading

Uh oh!

conventional-commit-lint-gcf bot commented Oct 13, 2022

Uh oh!

conventional-commit-lint-gcf bot commented Oct 13, 2022

Uh oh!

emmileaf commented Oct 13, 2022

Uh oh!

zhumin8 commented Oct 17, 2022

Uh oh!

emmileaf commented Oct 17, 2022

Uh oh!

zhumin8 left a comment

Uh oh!

zhumin8 Oct 17, 2022

Uh oh!

emmileaf Oct 18, 2022

Uh oh!

zhumin8 Oct 17, 2022

Uh oh!

sonarqubecloud bot commented Oct 18, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fix(Spring CodeGen): Update and move naming-related logic into utils #1061

fix(Spring CodeGen): Update and move naming-related logic into utils #1061

Uh oh!

Conversation

emmileaf commented Oct 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

conventional-commit-lint-gcf bot commented Oct 13, 2022

Uh oh!

conventional-commit-lint-gcf bot commented Oct 13, 2022

Uh oh!

emmileaf commented Oct 13, 2022

Uh oh!

zhumin8 commented Oct 17, 2022

Uh oh!

emmileaf commented Oct 17, 2022

Uh oh!

zhumin8 left a comment

Choose a reason for hiding this comment

Uh oh!

zhumin8 Oct 17, 2022

Choose a reason for hiding this comment

Uh oh!

emmileaf Oct 18, 2022

Choose a reason for hiding this comment

Uh oh!

zhumin8 Oct 17, 2022

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Oct 18, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

emmileaf commented Oct 13, 2022 •

edited

Loading