Rename methods and parameters for consistency across SDKs by arghya88 · Pull Request #410 · dapr/java-sdk

arghya88 · 2020-12-10T09:31:29Z

Signed-off-by: Arghya Sadhu arghya88@gmail.com

Description

Rename methods and parameters for consistency across SDKs

Issue reference

Fixes #409

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

Code compiles correctly
Created/updated tests
Extended the documentation

ghost · 2020-12-10T09:31:43Z

All CLA requirements met.

mukundansundar

@arghya88 Thanks for your contribution. I have left some comments in the PR.

artursouza · 2020-12-10T18:34:09Z

@arghya88 Thanks a lot for your contribution. The issue was not clear on every detail on the renaming, please see comments on the PR. We appreciate your help here.

arghya88 · 2020-12-11T07:45:59Z

@artursouza @mukundansundar thanks for the feedback. Have incorporated the changes.PTAL.

mukundansundar · 2020-12-11T07:55:47Z

@arghya88 Please remove all html changes. The docs will be updated during the release of the sdk.

mukundansundar

Rename GetStatesRequestBuilder also to GetBulkStateRequestBuilder
@arghya88 left a few more comments.

arghya88 · 2020-12-11T08:41:43Z

Rename GetStatesRequestBuilder also to GetBulkStateRequestBuilder
@arghya88 left a few more comments.

@mukundansundar Done

mukundansundar

@arghya88 Just a few more changes. Please resolve conflicts.
Rename stateStoreName to storeName in GetStateRequest, GetStateRequestBuilder, SaveStateRequest, SaveStateRequestBuilder, GetBulkState and GetBulkStateRequestBuilder.
Also change secretStoreName to storeName in GetSecretRequest and GetSecretRequestBuilder`

@artursouza any thoughts on these changes?

mukundansundar · 2020-12-12T07:44:58Z

+    InvokeServiceRequestBuilder builder = new InvokeServiceRequestBuilder(appId, methodName);
    InvokeServiceRequest req = builder
-        .withBody(request)
+        .withBody(data)


@artursouza Should body be renamed to data?
Also should InvokeServiceRequest and associated builder be renamed to InvokeMethodRequest since the API methods are being renamed?

@artursouza On second thought why is invokeService being renamed to invokeMethod? I think invokeService makes sense since we are invoking a particular method in that service.

Let's discuss this with the maintainers and see if we can change it. Let's not merge this PR for this milestone, so we have time to change it.

I would say let's do this in a followup PR after discussing with maintainers.

Well, we have cut the milestone already, so we can merge this PR soon and still have another PR to rename to invokeService again if maintainers agree to name it back.

artursouza · 2020-12-14T22:03:59Z

@arghya88 Please, address comments from @mukundansundar. Your PR is appreciated.

artursouza

Almost there.

arghya88 · 2020-12-16T04:22:12Z

Almost there.

@artursouza let me know if anything else is needed.

Signed-off-by: Arghya Sadhu <arghya88@gmail.com>

codecov · 2020-12-16T07:47:22Z

Codecov Report

Merging #410 (d15893b) into master (de4c37e) will increase coverage by 0.00%.
The diff coverage is 92.03%.

@@            Coverage Diff            @@
##             master     #410   +/-   ##
=========================================
  Coverage     83.26%   83.27%           
  Complexity      835      835           
=========================================
  Files            72       72           
  Lines          2486     2487    +1     
  Branches        255      255           
=========================================
+ Hits           2070     2071    +1     
  Misses          293      293           
  Partials        123      123

Impacted Files	Coverage Δ	Complexity Δ
...ain/java/io/dapr/actors/client/DaprGrpcClient.java	`87.50% <ø> (ø)`	`6.00 <0.00> (ø)`
...ain/java/io/dapr/actors/client/DaprHttpClient.java	`100.00% <ø> (ø)`	`3.00 <0.00> (ø)`
...in/java/io/dapr/actors/runtime/DaprGrpcClient.java	`95.50% <ø> (ø)`	`20.00 <0.00> (ø)`
...in/java/io/dapr/actors/runtime/DaprHttpClient.java	`80.70% <ø> (ø)`	`13.00 <0.00> (ø)`
...dapr/client/domain/GetBulkStateRequestBuilder.java	`84.21% <84.21%> (ø)`	`3.00 <3.00> (?)`
...c/main/java/io/dapr/client/AbstractDaprClient.java	`89.85% <84.61%> (+0.14%)`	`33.00 <28.00> (ø)`
...ain/java/io/dapr/actors/client/ActorProxyImpl.java	`90.24% <100.00%> (ø)`	`24.00 <6.00> (ø)`
...ain/java/io/dapr/actors/runtime/AbstractActor.java	`85.33% <100.00%> (ø)`	`27.00 <2.00> (ø)`
...io/dapr/actors/runtime/DaprStateAsyncProvider.java	`60.86% <100.00%> (ø)`	`14.00 <2.00> (ø)`
...k/src/main/java/io/dapr/client/DaprClientGrpc.java	`85.61% <100.00%> (ø)`	`84.00 <23.00> (ø)`
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update de4c37e...0b649fa. Read the comment docs.

arghya88 force-pushed the rename-methods branch 2 times, most recently from 0f3094b to ee9576a Compare December 10, 2020 10:51

mukundansundar reviewed Dec 10, 2020

View reviewed changes

mukundansundar requested a review from artursouza December 10, 2020 18:17

arghya88 force-pushed the rename-methods branch 3 times, most recently from f85bc97 to 607be1b Compare December 11, 2020 07:05

mukundansundar reviewed Dec 11, 2020

View reviewed changes

Comment thread sdk/src/main/java/io/dapr/client/DaprClient.java Outdated

Comment thread sdk/src/test/java/io/dapr/client/DaprClientGrpcTest.java Outdated

arghya88 force-pushed the rename-methods branch 3 times, most recently from c4181f5 to 2a4bfec Compare December 11, 2020 08:30

mukundansundar reviewed Dec 12, 2020

View reviewed changes

Comment thread sdk/src/main/java/io/dapr/client/domain/GetBulkStateRequestBuilder.java Outdated

Comment thread sdk/src/main/java/io/dapr/client/domain/GetBulkStateRequest.java Outdated

Comment thread sdk/src/main/java/io/dapr/client/DaprClient.java Outdated

mukundansundar reviewed Dec 12, 2020

View reviewed changes

arghya88 force-pushed the rename-methods branch from 2a4bfec to a028a4e Compare December 12, 2020 08:30

artursouza reviewed Dec 14, 2020

View reviewed changes

Comment thread examples/src/main/java/io/dapr/examples/state/StateClient.java

arghya88 force-pushed the rename-methods branch 3 times, most recently from 14780a0 to ab917c8 Compare December 15, 2020 06:23

artursouza reviewed Dec 15, 2020

View reviewed changes

Comment thread examples/src/main/java/io/dapr/examples/state/README.md Outdated

artursouza reviewed Dec 15, 2020

View reviewed changes

arghya88 force-pushed the rename-methods branch from ab917c8 to 4d7f645 Compare December 16, 2020 04:21

Rename methods and parameters for consistency across SDKs

0b649fa

Signed-off-by: Arghya Sadhu <arghya88@gmail.com>

arghya88 force-pushed the rename-methods branch from 338da40 to 0b649fa Compare December 16, 2020 07:40

artursouza approved these changes Dec 16, 2020

View reviewed changes

artursouza merged commit 11d9e3b into dapr:master Dec 16, 2020

arghya88 deleted the rename-methods branch December 23, 2020 12:25

Uh oh!

Conversation

arghya88 commented Dec 10, 2020

Description

Issue reference

Checklist

Uh oh!

ghost commented Dec 10, 2020 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mukundansundar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

artursouza commented Dec 10, 2020

Uh oh!

arghya88 commented Dec 11, 2020

Uh oh!

mukundansundar commented Dec 11, 2020

Uh oh!

mukundansundar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

arghya88 commented Dec 11, 2020

Uh oh!

mukundansundar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mukundansundar Dec 12, 2020

Choose a reason for hiding this comment

Uh oh!

mukundansundar Dec 12, 2020

Choose a reason for hiding this comment

Uh oh!

artursouza Dec 14, 2020

Choose a reason for hiding this comment

Uh oh!

arghya88 Dec 15, 2020

Choose a reason for hiding this comment

Uh oh!

artursouza Dec 15, 2020

Choose a reason for hiding this comment

Uh oh!

artursouza commented Dec 14, 2020

Uh oh!

Uh oh!

Uh oh!

artursouza left a comment

Choose a reason for hiding this comment

Uh oh!

arghya88 commented Dec 16, 2020

Uh oh!

codecov Bot commented Dec 16, 2020

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ghost commented Dec 10, 2020 •

edited by ghost

Loading