8000 fix(spring): fix client and settings class names in comments by emmileaf · Pull Request #1277 · googleapis/sdk-platform-java · GitHub
[go: up one dir, main page]
Skip to content

fix(spring): fix client and settings class names in comments #1277

< 8000 div class="gh-header-actions mt-0 mb-3 mb-md-2 ml-1 flex-md-order-1 flex-shrink-0 d-flex flex-items-center gap-1">
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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jan 26, 2023
Merged

fix(spring): fix client and settings class names in comments #1277

merged 2 commits into from
Jan 26, 2023

Conversation

emmileaf
Copy link
Contributor
@emmileaf emmileaf commented Jan 25, 2023 •
edited
Loading

This PR addresses the javadoc error observed in our attempt to set up spring-cloud-previews for release.

Cause of javadoc error: few exceptions of incorrect client library <service>Client and <service>Settings class names

  • Misalignment: service.name() vs. service.overridenName() can be different
  • This did not raise compilation issues in the generated code, because we used gapic-generator-java’s ClassNames.getServiceClientClassName(service), which correctly aligns with the client library class name.
  • service.name() + “Client” was used in other places such as javadoc comments and spring-specific methods and classnames. Turns out this assumption does not hold true.

@emmileaf emmileaf added the spring pr that's related to spring code gen, intend to merge into autoconfig-gen-draft2 branch. label Jan 25, 2023
@emmileaf emmileaf mentioned this pull request Jan 25, 2023
Merged
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@emmileaf emmileaf marked this pull request as ready for review January 25, 2023 22:39
@emmileaf emmileaf requested a review from a team as a code owner January 25, 2023 22:39
diegomarquezp
diegomarquezp approved these changes Jan 26, 2023
View reviewed changes
Copy link
Contributor
@diegomarquezp diegomarquezp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for ensuring consistency through ClassNames.
A test that compiles comments for one of the failed classes (e.g. AccessApproval[Admin]Client) and confirms the javadoc links are valid would be a nice to have.

@emmileaf
Copy link
Contributor Author

A test that compiles comments for one of the failed classes (e.g. AccessApproval[Admin]Client) and confirms the javadoc links are valid would be a nice to have.

@diegomarquezp Agreed - I'll merge this PR for unblocking, and follow-up on this suggestion. Were you thinking of some sort of coverage on the generator side (through golden files), or having an earlier check for javadoc links in the spring-cloud-gcp generated code ?

@emmileaf emmileaf merged commit 26c2e12 into autoconfig-gen-draft2 Jan 26, 2023
@emmileaf emmileaf deleted the javadoc-comment-patch branch January 26, 2023 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@meltsufin meltsufin meltsufin approved these changes

@diegomarquezp diegomarquezp diegomarquezp approved these changes

@blakeli0 blakeli0 Awaiting requested review from blakeli0

Assignees
No one assigned
Labels
spring pr that's related to spring code gen, intend to merge into autoconfig-gen-draft2 branch.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@emmileaf @meltsufin @diegomarquezp
0