-
Notifications
You must be signed in to change notification settings - Fork 78
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
docs: label legacy storage classes in documentation #267
docs: label legacy storage classes in documentation #267
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.
Thanks for picking this up @dmitry-fa
*/ | ||
public static final StorageClass REGIONAL = type.createAndRegister("REGIONAL"); | ||
|
||
/** | ||
* Multi-regional storage class. This is supported as a legacy storage class and will be | ||
* deprecated in the future. See: https://cloud.google.com/storage/docs/storage-classes for | ||
* details | ||
* deprecated in the future. This class cannot be set. Unless you already are using it, you should |
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.
Legacy storage classes can still be set, please remove This class cannot be set
for all legacy storage classes.
Additionally, the issue that didn't across in my issue is that a developer at first only see's Mult-regional storage class.
and not the rest.
The additional information isn't available until scrolling down:
If it's being truncated because of the period .
. then I recommend this:
Legacy Multi-regional storage class, use
STANDARD
storage class for new buckets.
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.
Legacy Multi-regional storage class, use REGIONAL storage class for new buckets.
- Do you really mean REGIONAL?
https://cloud.google.com/storage/docs/storage-classes#legacy recommends STANDARD class:
Unless you already are using one of these additional classes, you should use Standard Storage instead.
- A storage class can be set not only for buckets, but for object as well. And not only for new, but for existing. Right? May be it's not correct to say
for new buckets
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.
🤦♂️ I meant STANDARD.
Good catch (blobs vs buckets), then use:
Legacy {Storage class} storage class, use STANDARD storage class instead.
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.
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.
Thanks @dmitry-fa, LGTM. Rerunning tests due to flake.
Codecov Report
@@ Coverage Diff @@
## master #267 +/- ##
=========================================
Coverage 63.52% 63.52%
Complexity 540 540
=========================================
Files 30 30
Lines 4776 4776
Branches 431 431
=========================================
Hits 3034 3034
Misses 1580 1580
Partials 162 162
Continue to review full report at Codecov.
|
Fixes #254