Skip to content

Expose MethodRouter call_with_state - #3860

Open
Manishakumari21 wants to merge 2 commits into
tokio-rs:mainfrom
Manishakumari21:fix-method-router-shared-reference
Open

Expose MethodRouter call_with_state#3860
Manishakumari21 wants to merge 2 commits into
tokio-rs:mainfrom
Manishakumari21:fix-method-router-shared-reference

Conversation

@Manishakumari21

Copy link
Copy Markdown

Motivation

Solution

@Manishakumari21

Copy link
Copy Markdown
Author

Hi, I’ve implemented the requested change by exposing MethodRouter::call_with_state and added a regression test covering shared-reference usage.

I also verified the change with the relevant test, which passes successfully. Thank you for reviewing!

@mladedav

Copy link
Copy Markdown
Collaborator

Could you link the issue where the change was requested please?

Also, I don't think the test actually tests what you say. It's a submodule so the visibility is not tested at all and the test itself is named as if it is supposed to test that this works with shared reference, but if the method required an owned object, I think it would still pass.

Also next time, you should write any information you want to share about the PR in the description (including motivation and the issue link) not into comments. It's better visible for anyone seeing the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants