Skip to content
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

fix: cleanup @BetaApi from Resource Name Builder Methods #2450

Merged
merged 11 commits into from
Feb 16, 2024
Merged

Conversation

zhumin8
Copy link
Contributor

@zhumin8 zhumin8 commented Feb 8, 2024

fixes #2099

@zhumin8 zhumin8 requested a review from a team as a code owner February 8, 2024 16:05
@product-auto-label product-auto-label bot added the size: s Pull request size is small. label Feb 8, 2024
@zhumin8 zhumin8 marked this pull request as draft February 8, 2024 16:57
@product-auto-label product-auto-label bot added size: m Pull request size is medium. and removed size: s Pull request size is small. labels Feb 8, 2024
@zhumin8 zhumin8 marked this pull request as ready for review February 8, 2024 19:42
@lqiu96
Copy link
Contributor

lqiu96 commented Feb 9, 2024

Should this one be removed as well?

@BetaApi("The per-pattern Builders are not stable yet and may be changed in the future.")
from
// Return the class.
AnnotationNode betaAnnotation =
AnnotationNode.builder()
.setType(FIXED_TYPESTORE.get("BetaApi"))
.setDescription(
"The per-pattern Builders are not stable yet and may be changed in the future.")
.build();
List<AnnotationNode> classAnnotations =
isDefaultClass ? Collections.emptyList() : Arrays.asList(betaAnnotation);

@zhumin8
Copy link
Contributor Author

zhumin8 commented Feb 12, 2024

Should this one be removed as well?

@BetaApi("The per-pattern Builders are not stable yet and may be changed in the future.")

Yes. Thanks for catching this.

@zhumin8
Copy link
Contributor Author

zhumin8 commented Feb 12, 2024

I am a bit confused. I followed this guide to update golden files for showcase, and showcase ci tests are passing.
But I noticed there are still appearances of "per-pattern builders" under showcase folder:

@BetaApi("The per-pattern Builders are not stable yet and may be changed in the future.")
public static Builder newUserLegacyUserBlurbBuilder() {
return new Builder();
}
@BetaApi("The per-pattern Builders are not stable yet and may be changed in the future.")
public static UserBlurbBuilder newUserBlurbBuilder() {
return new UserBlurbBuilder();
}
@BetaApi("The per-pattern Builders are not stable yet and may be changed in the future.")
public static RoomBlurbBuilder newRoomBlurbBuilder() {
return new RoomBlurbBuilder();
}
@BetaApi("The per-pattern Builders are not stable yet and may be changed in the future.")

@lqiu96 I am missing anything in my workflow? Or is this class not part of showcase golden?

@lqiu96
Copy link
Contributor

lqiu96 commented Feb 12, 2024

I am a bit confused. I followed this guide to update golden files for showcase, and showcase ci tests are passing. But I noticed there are still appearances of "per-pattern builders" under showcase folder:

@BetaApi("The per-pattern Builders are not stable yet and may be changed in the future.")
public static Builder newUserLegacyUserBlurbBuilder() {
return new Builder();
}
@BetaApi("The per-pattern Builders are not stable yet and may be changed in the future.")
public static UserBlurbBuilder newUserBlurbBuilder() {
return new UserBlurbBuilder();
}
@BetaApi("The per-pattern Builders are not stable yet and may be changed in the future.")
public static RoomBlurbBuilder newRoomBlurbBuilder() {
return new RoomBlurbBuilder();
}
@BetaApi("The per-pattern Builders are not stable yet and may be changed in the future.")

@lqiu96 I am missing anything in my workflow? Or is this class not part of showcase golden?

I suspect that you made gapic-generator changes and need to re-run mvn clean install. I think showcase is pointed to use the SNAPSHOT version and the version it's using is the pre-generator changes. Can you try re-installing?

Edit: Oh, I just saw your comment about showcase CI tests passing. Can you try clearing the GH CI cache?

@lqiu96
Copy link
Contributor

lqiu96 commented Feb 12, 2024

Ah one more thing, can you double check if the FIXED_TYPESTORE has BetaApi.class reference in there? If all the betaapi references are removed from this composer, can you remove it from there as well?

@zhumin8
Copy link
Contributor Author

zhumin8 commented Feb 13, 2024

@lqiu96 @burkedavison can you take another look?

zhumin8 added a commit that referenced this pull request Feb 13, 2024
ddixit14 pushed a commit that referenced this pull request Feb 15, 2024
Copy link

Quality Gate Passed Quality Gate passed for 'gapic-generator-java-root'

Issues
0 New issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

Copy link

Quality Gate Passed Quality Gate passed for 'java_showcase_integration_tests'

Issues
0 New issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@zhumin8 zhumin8 merged commit 6e8d098 into main Feb 16, 2024
25 checks passed
@zhumin8 zhumin8 deleted the fix-2099 branch February 16, 2024 16:38
zhumin8 pushed a commit that referenced this pull request Feb 29, 2024
🤖 I have created a release *beep* *boop*
---


<details><summary>2.36.0</summary>

##
[2.36.0](v2.35.0...v2.36.0)
(2024-02-29)


### Features

* check library_name is unique among libraries
([#2490](#2490))
([8123f0b](8123f0b))


### Bug Fixes

* cleanup @BetaApi from Resource Name Builder Methods
([#2450](#2450))
([6e8d098](6e8d098)),
closes
[#2099](#2099)
* Fix watchdog to start with WAITING state
([#2468](#2468))
([dedc40f](dedc40f))
* ignore comment in BUILD
([#2492](#2492))
([6ca20e5](6ca20e5))
* remove @BetaApi from ApiFutures and ApiService
([#2454](#2454))
([f59e717](f59e717)),
closes
[#2098](#2098)


### Dependencies

* grandfathering the dependencies for java-pubsublite and java-bigquery
([#2504](#2504))
([9ceab23](9ceab23))
* update dependency gradle to v7.6.4
([#2474](#2474))
([607dc59](607dc59))
* update dependency org.graalvm.sdk:graal-sdk to v22.3.5
([#2475](#2475))
([2de487b](2de487b))
* update grpc dependencies to v1.62.2
([#2506](#2506))
([f438603](f438603))


### Documentation

* Add contribution guidelines.
([#2045](#2045))
([9939b43](9939b43))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: m Pull request size is medium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

generator: Remove @BetaApi from Resource Name Builder Methods
2 participants