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: Adding ability to create a subscription at HEAD #545
feat: Adding ability to create a subscription at HEAD #545
Conversation
Codecov Report
@@ Coverage Diff @@
## master #545 +/- ##
============================================
- Coverage 72.70% 72.66% -0.04%
- Complexity 860 861 +1
============================================
Files 156 156
Lines 4477 4482 +5
Branches 210 211 +1
============================================
+ Hits 3255 3257 +2
- Misses 1102 1104 +2
- Partials 120 121 +1
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 after fixes. Ping me once fixed
@@ -108,6 +119,17 @@ static AdminClient create(AdminClientSettings settings) throws ApiException { | |||
*/ | |||
ApiFuture<Subscription> createSubscription(Subscription subscription); |
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.
default ApiFuture<Subscription> createSubscription(Subscription subscription) { createSubscription(subscription, OffsetLocation.END); }
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.
Also modify the comment to make it clear where this creates the subscription
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.
Done. This is ready for another look. Thanks!
No description provided.