-
Notifications
You must be signed in to change notification settings - Fork 2k
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: (data_catalog) migrate region tag in deleteFilesetEntry.js #3949
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi team,
I'm currently reviewing this pull request and will provide a full review shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed:
This pull request, submitted by @hivanalejandro, aims to fix a bug (b/389895440) by migrating a region tag in the deleteFilesetEntry.js
file within the datacatalog/quickstart
directory.
Specifically, the changes involve updating the region tag from datacatalog_delete_fileset_quickstart_tag
to data_catalog_delete_fileset_quickstart_tag
.
Here's a breakdown of the changes:
- File:
datacatalog/quickstart/deleteFilesetEntry.js
- Changes: The region tag was updated on lines 29 and 74. The
data_catalog_delete_fileset_quickstart_tag
was added on line 29, and the same tag was added on line 76.
- Changes: The region tag was updated on lines 29 and 74. The
The author has indicated that all tests and linting checks have passed. The pull request is ready for merging once approved.
A little haiku to lighten the mood:
Tags shift, code flows free,
Bugs vanish, a cleaner scene,
Joy in the merge.
Thanks,
Gemini
Here is the summary of changes. You are about to add 1 region tag.
This comment is generated by snippet-bot.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change correctly migrates the region tag as requested. However, the original region tag should be removed entirely instead of being duplicated.
// [START data_catalog_delete_fileset_quickstart_tag] | ||
// [START datacatalog_delete_fileset_quickstart_tag] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The old region tag datacatalog_delete_fileset_quickstart_tag
should be removed instead of being duplicated. The correct region tag is data_catalog_delete_fileset_quickstart_tag
.
// [START data_catalog_delete_fileset_quickstart_tag] | |
// [START datacatalog_delete_fileset_quickstart_tag] | |
// [START data_catalog_delete_fileset_quickstart_tag] | |
// [END datacatalog_delete_fileset_quickstart_tag] | ||
// [END data_catalog_delete_fileset_quickstart_tag] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Description
Migrate region "datacatalog_delete_fileset_quickstart_tag" to "data_catalog_delete_fileset_quickstart_tag"
Fixes b/389895440
Note: Before submitting a pull request, please open an issue for discussion if you are not associated with Google.
Checklist
npm test
(see Testing)npm run lint
(see Style)GoogleCloudPlatform/nodejs-docs-samples
. Not a fork.