-
Notifications
You must be signed in to change notification settings - Fork 80
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: Documentation for Blob.update() and Storage.update() methods is confusing/incorrect #261
Conversation
…confusing/incorrect
…confusing/incorrect
google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java
Outdated
Show resolved
Hide resolved
google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java
Outdated
Show resolved
Hide resolved
google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java
Outdated
Show resolved
Hide resolved
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.
two nits, thanks @dmitry-fa!
* @param options update options | ||
* @return a {@code Blob} object with updated information | ||
* @param options preconditions to apply the update | ||
* @return the updated blob |
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 updated {@code Blob}
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.
okay
* the metadata generation of the current blob. If you want to update the information only if the | ||
* current blob metadata are at their latest version use the {@code metagenerationMatch} option: | ||
* {@code newBlob.update(BlobTargetOption.metagenerationMatch())}. | ||
* Updates the blob properties. The {@code options} parameter contains the preconditions for |
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'd state {@code options}
is BlobTargetOption
and instead of preconditions
use request options
and link to update API documentation: https://cloud.google.com/storage/docs/json_api/v1/objects/update
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.
and link to update API documentation: https://cloud.google.com/storage/docs/json_api/v1/objects/update
done
I'd state {@code options} is BlobTargetOption
I think this is clear from the parameter type
and instead of preconditions use request options
Personally for me it was hard to understand the purspose of this parameter. So, I tried to rephrase this statement to help users not very familiar with the Storage details to get the meaning. I came up to: The options parameter contains the preconditions for applying the update
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, sgtm, I think having the link will help provide information.
Codecov Report
@@ Coverage Diff @@
## master #261 +/- ##
============================================
- Coverage 63.52% 63.46% -0.07%
Complexity 540 540
============================================
Files 30 30
Lines 4776 4776
Branches 431 431
============================================
- Hits 3034 3031 -3
- Misses 1580 1583 +3
Partials 162 162
Continue to review full report at Codecov.
|
Fixes #252