-
Notifications
You must be signed in to change notification settings - Fork 71
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
feat: update V4 signature to pass conformance tests #114
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.
Nicely done, overall LGTM I have one question before approving.
@@ -1038,6 +1053,7 @@ public static BlobListOption fields(BlobField... fields) { | |||
HOST_NAME, | |||
PATH_STYLE, | |||
VIRTUAL_HOSTED_STYLE, | |||
BUCKET_BOUND_HOST_NAME, |
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.
Jesse, I want to confirm that HOST_NAME can't be in the same way as BUCKET_BOUND_HOST_NAME. Is this accurate?
Codecov Report
@@ Coverage Diff @@
## master #114 +/- ##
============================================
+ Coverage 63.3% 63.73% +0.42%
- Complexity 528 539 +11
============================================
Files 30 30
Lines 4720 4740 +20
Branches 450 455 +5
============================================
+ Hits 2988 3021 +33
+ Misses 1571 1564 -7
+ Partials 161 155 -6
Continue to review full report at Codecov.
|
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.
LGTM
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.
LGTM.
This updates the conformance tests and adds the consistency changes necessary to make them pass, as per #84 and #85