-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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(gae): migrate regions adding gae_storage prefix #5070
base: main
Are you sure you want to change the base?
fix(gae): migrate regions adding gae_storage prefix #5070
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 my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly understand the changes:
This pull request, submitted by OremGLG, aims to migrate regions and associate them with an official product. The core change involves adding the gae_storage
prefix to various sections within the docs/appengine/storage/app.go
file. This appears to be a refactoring effort to improve organization and potentially clarify the code's purpose.
Here's a breakdown of the changes:
- File Modified:
docs/appengine/storage/app.go
- Changes: The primary modifications involve adding
gae_storage
prefixes to several comment blocks, specifically around import statements, functions for getting the default bucket, writing, reading, copying files, and listing buckets. This suggests a more structured approach to code comments and potentially aligns with a new product or naming convention.- Lines 18-19: Added
//[START gae_storage_imports]
and//[START imports]
- Lines 36-37: Added
//[END imports]
and//[END gae_storage_imports]
- Lines 72-73: Added
//[START gae_storage_get_default_bucket]
and//[START get_default_bucket]
- Lines 80-81: Added
//[END get_default_bucket]
and//[END gae_storage_get_default_bucket]
- Lines 133-134: Added
//[START gae_storage_write]
and//[START write]
- Lines 161-162: Added
//[END write]
and//[END gae_storage_write]
- Lines 164-165: Added
//[START gae_storage_read]
and//[START read]
- Lines 190-191: Added
//[END read]
and//[END gae_storage_read]
- Lines 193-194: Added
//[START gae_storage_copy]
and//[START copy]
- Lines 210-211: Added
//[END copy]
and//[END gae_storage_copy]
- Lines 252-253: Added
//[START gae_storage_list_bucket]
and//[START list_bucket]
- Lines 273-274: Added
//[END list_bucket]
and//[END gae_storage_list_bucket]
- Lines 18-19: Added
The pull request also includes a checklist to ensure code quality and adherence to guidelines.
I'll provide a more detailed review shortly.
Meanwhile, a little verse to brighten your day:
Code flows like a river,
Prefixes guide its clear path,
Clarity's gentle grace.
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.
This pull request migrates regions to associate them with an official product by adding the gae_storage
prefix. The changes are well-structured and easy to understand. The approach of adding specific start and end tags for each region makes it clear which code blocks belong to which part of the documentation. The checklist is mostly complete, but needs the issue number added.
docs/appengine/storage/app.go
Outdated
//[START gae_storage_imports] | ||
//[START imports] |
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.
docs/appengine/storage/app.go
Outdated
@@ -67,6 +69,7 @@ | |||
return | |||
} | |||
|
|||
//[START gae_storage_get_default_bucket] |
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.
docs/appengine/storage/app.go
Outdated
@@ -126,6 +130,7 @@ | |||
} | |||
} | |||
|
|||
// [START gae_storage_write] |
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.
docs/appengine/storage/app.go
Outdated
|
||
// [START gae_storage_read] |
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.
docs/appengine/storage/app.go
Outdated
|
||
// [START gae_storage_copy] |
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.
docs/appengine/storage/app.go
Outdated
@@ -239,6 +249,7 @@ | |||
} | |||
} | |||
|
|||
// [START gae_storage_list_bucket] |
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.
No region tags are edited in this PR.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.
Make sure documentation is updated accordingly.
It is a frozen sample, according to @iennae "Migrate the region tag in the 'main' branch, but don't update frozen samples in those cases Eg: GoogleCloudPlatform/python-docs-samples#13047 That's why I am not updating documentation. |
Description
Migrate regions in order to create an associate them with an oficial product
Fixes b/389070563
Note: Before submitting a pull request, please open an issue for discussion if you are not associated with Google.
Checklist
go test -v ./..
(see Testing)gofmt
(see Formatting)go vet
(see Formatting)