-
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 #3947
Conversation
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.
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 understand the changes:
This pull request, submitted by @hivanalejandro, aims to migrate the region tag in the nodejs-docs-samples
repository's datacatalog
quickstart sample. Specifically, it addresses issue b/389763067.
The changes involve updating the createFilesetEntry.js
file:
- The old region tag
datacatalog_create_fileset_quickstart_tag
is replaced with the new tagdata_catalog_create_fileset_quickstart_tag
. - This change is reflected both at the beginning and end of the
main
function (lines 29-30 and 128-129 respectively).
The author has indicated that all tests and linting checks have passed. The checklist in the PR description confirms this.
I'll provide a more detailed review shortly, but this summary should give everyone a good overview of the changes.
A little poem to brighten your day:
Tags shift, a code's new hue,
A region's name, refreshed and true,
Bugs flee, the fix is done.
Best,
Gemini/code-review-assist
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 region tag was duplicated in the before patch, this has been corrected. Also, the checklist should be completed before requesting a merge.
@@ -26,6 +26,7 @@ const main = async ( | |||
entryGroupId, | |||
entryId | |||
) => { | |||
// [START data_catalog_create_fileset_quickstart_tag] | |||
// [START datacatalog_create_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.
}; | ||
// [END datacatalog_create_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_create_fileset_quickstart_tag" to "data_catalog_create_fileset_quickstart_tag"
Fixes b/389763067
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.