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.

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

feat: [vmwareengine] new module for vmwareengine #8837

Merged
merged 4 commits into from Nov 29, 2022
Merged

feat: [vmwareengine] new module for vmwareengine #8837

merged 4 commits into from Nov 29, 2022

Conversation

lqiu96
Copy link
Contributor

@lqiu96 lqiu96 commented Nov 22, 2022 •
edited

New Client has been generated with this command:

python3.9 generation/new_client/new-client.py generate \
  --api_shortname=vmwareengine \
  --proto-path=google/cloud/vmwareengine \
  --name-pretty="Google Cloud VMware Engine" \
  --product-docs="https://cloud.google.com/vmware-engine/" \
  --api-description="Easily lift and shift your VMware-based applications to Google Cloud without changes to your apps, tools, or processes."

@lqiu96
Copy link
Contributor Author

lqiu96 commented Nov 22, 2022

This doesn't look like the intended behavior. Not sure why it's updating the pom.xml for other modules.

suztomo
suztomo reviewed Nov 22, 2022
View reviewed changes
java-accessapproval/google-cloud-accessapproval-bom/pom.xml Outdated
@@ -9,7 +9,7 @@
<parent>
<groupId>com.google.cloud</groupId>
<artifactId>google-cloud-pom-parent</artifactId>
<version>1.1.0-SNAPSHOT</version><!-- {x-version-update:google-cloud-java:current} -->
<version>1.1.0-SNAPSHOT</version><!-- {x-version-update:google-cloud-pom-parent:current} -->
Copy link
Member

@suztomo suztomo Nov 22, 2022 •
edited

Choose a reason for hiding this comment

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

@ddixit14 1 or more scripts need to be updated to use google-cloud-java tag. Can you identify which scripts to be updated and create a pull request?

Copy link
Contributor

Choose a reason for hiding this comment

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

This PR shouldn't update this file. Right?

Copy link
Contributor Author

@lqiu96 lqiu96 Nov 29, 2022 •
edited

Choose a reason for hiding this comment

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

I think it's updating the {x-version-update: ... :current} tag via

google-cloud-java/generation/new_client/new-client.py

Line 270 in 998d5f3

"bash", "generation/set_parent_pom.sh"

I think this should update it correctly: #8848.

@lqiu96 lqiu96 added the owlbot:run Add this label to trigger the Owlbot post processor. label Nov 29, 2022
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Nov 29, 2022
@lqiu96 lqiu96 marked this pull request as ready for review November 29, 2022 16:44
lqiu96
lqiu96 commented Nov 29, 2022
View reviewed changes
java-vmwareengine/.repo-metadata.json
"release_level": "preview",
"transport": "grpc",
"language": "java",
"repo": "googleapis/java-vmwareengine",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this should be googleapis/google-cloud-java

Copy link
Member

Choose a reason for hiding this comment

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

I feel this can be fixed later. Do you see this field in JSON used somewhere?

Copy link
Contributor Author

@lqiu96 lqiu96 Nov 29, 2022 •
edited

Choose a reason for hiding this comment

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

Ok, I'll create an issue for this then. Issue: #8849

@lqiu96 lqiu96 mentioned this pull request Nov 29, 2022
Closed
suztomo
suztomo requested changes Nov 29, 2022
View reviewed changes
java-vmwareengine/README.md Outdated
@@ -0,0 +1,201 @@
# Google Google Cloud VMware Engine Client for Java
Copy link
Member

Choose a reason for hiding this comment

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

Typo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I copied the display name value on drift:
image

Should I change it to Cloud VMware Engine?

Copy link
Member

Choose a reason for hiding this comment

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

Can you change this line "Google Cloud VMware Engine Client for Java"? No need to re-run the script.

Copy link
Contributor Author

@lqiu96 lqiu96 Nov 29, 2022 •
edited

Choose a reason for hiding this comment

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

Wouldn't it get overwritten the next time owlbot post processor runs? I thought the README template is from: https://github.com/googleapis/synthtool/blob/934c51d74f1ec59a20892525248de1bed8fec638/synthtool/gcp/templates/java_library/README.md?plain=1#L5

Hmm, actually I can test this and see...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, post processor doesn't overwrite this. I assumed the post-processor created the README via synthtool: https://github.com/googleapis/google-cloud-java/blob/0f71f527a7e5c9812767b3c9e0820bc033b57510/generation/new_client/new-client.py#LL223C12-L223C12

If not, how was the README.md created?

@lqiu96 lqiu96 added the owlbot:run Add this label to trigger the Owlbot post processor. label Nov 29, 2022
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Nov 29, 2022
@lqiu96 lqiu96 merged commit d61b54a into main Nov 29, 2022
@lqiu96 lqiu96 deleted the new_module_java-vmwareengine branch November 29, 2022 20:30
@release-please release-please bot mentioned this pull request Nov 29, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ddixit14 ddixit14 ddixit14 left review comments

@suztomo suztomo suztomo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@lqiu96 @suztomo @ddixit14