-
Notifications
You must be signed in to change notification settings - Fork 345
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
adding get actor reminder API in docs #1113
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1113 +/- ##
=======================================
Coverage 67.16% 67.16%
=======================================
Files 170 170
Lines 5695 5695
Branches 607 607
=======================================
Hits 3825 3825
Misses 1727 1727
Partials 143 143
Flags with carried forward coverage won't be shown. Click here to find out more. |
public async Task<IActorReminder> GetReminder() | ||
{ | ||
await this.GetReminderAsync("MyReminder"); | ||
} |
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.
Personal preference, we should include this with an input string as an example. Otherwise people may think it can only take hardcoded names.
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 kept this as to be consistent with other APIs where the reminder-name is hardcoded in RegisterReminder
and UnregisterReminder
. Both of them take the reminder name as inputs as well, similar to GetReminder
.
https://github.com/dapr/dotnet-sdk/pull/1113/files/53876be8bf17e98bb9011094ab39623dc0be74e4#diff-763693d0f8815d098d3c9e4f284e8909335c17756bf549eef7abc550c3151a63L213
https://github.com/dapr/dotnet-sdk/pull/1113/files/53876be8bf17e98bb9011094ab39623dc0be74e4#diff-763693d0f8815d098d3c9e4f284e8909335c17756bf549eef7abc550c3151a63L225
It's also same as kept in examples: https://github.com/dapr/dotnet-sdk/blob/master/test/Dapr.E2E.Test.App/Actors/ReminderActor.cs
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.
Is this okay @halspang ?
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.
Small change in the example.
Signed-off-by: Shivam Kumar <shivamkm07@gmail.com>
Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> Signed-off-by: Erik O'Leary <erik.m.oleary@gmail.com>
Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> Signed-off-by: Divya Perumal <diperuma@microsoft.com>
Description
It adds the get actor reminder API in docs. The API implementation is merged in release-1.11 in #1103.
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: #[issue number]
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: