-
Notifications
You must be signed in to change notification settings - Fork 224
Renaming streaming subscription to "subscribeToTopic" #1626
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
base: master
Are you sure you want to change the base?
Conversation
|
@dapr/approvers-java-sdk and @dapr/maintainers-java-sdk could you please take a look. I think this should be the final name for streaming subscription and it aligns with other SDKs nicely. @alicejgibbons @javier-aliaga @cicoyle could you also please take a look. Thank you 🙇 |
cicoyle
left a comment
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.
Can we create an alias for this so we dont break users. This functionality was backported, so I think its best to add an alias and mark the old as deprecated to be removed later.
Signed-off-by: Artur Ciocanu <artur.ciocanu@gmail.com>
Signed-off-by: Artur Ciocanu <artur.ciocanu@gmail.com>
Signed-off-by: Artur Ciocanu <artur.ciocanu@gmail.com>
c802e76 to
49a4b03
Compare
|
@cicoyle @salaboy and @javier-aliaga I have added aliases so we keep backward compatibility. |
|
@artur-ciocanu ok.. so there is a codecov gap there.. I promise you, I will fix the reports soon, but basically, you need to keep a couple of tests for the previous signature of the method. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1626 +/- ##
============================================
- Coverage 78.99% 78.95% -0.05%
- Complexity 2096 2097 +1
============================================
Files 228 228
Lines 6322 6324 +2
Branches 693 693
============================================
- Hits 4994 4993 -1
- Misses 976 980 +4
+ Partials 352 351 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
salaboy
left a comment
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, it just need the codecov report green
Description
Renaming streaming subscription to "subscribeToTopic". This should align better with other Dapr SDKs and ensure that is more ergonomic.
Issue reference
We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.
Please reference the issue this PR will close: N/A
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: