Skip to content

Conversation

@nerfZael
Copy link
Contributor

@nerfZael nerfZael commented May 15, 2023

The goal of this PR is to make the wrapper trait, specifically the invoke method, not coupled with the resolution context.
Resolution context passing has already been achieved through the subinvoker construct (This part was implemented in a previous PR: #94). Which means the resolution context argument is now obsolete.

Additionally, the invoke_wrapper method signature has been changed to:

 fn invoke_wrapper_raw(
    &self,
+   wrapper: &dyn Wrapper,
    uri: &Uri,
    method: &str,
    args: Option<&[u8]>,
    env: Option<&Env>,
    resolution_context: Option<&mut UriResolutionContext>,
) -> Result<Vec<u8>, Error>;

The wrapper argument has been made a reference since there was no need to keep it as Arc after the subinvoker implementation (which is an Arc itself).

Less notable changes:

  • Both invoke_wrapper and invoke_raw in the PolywrapClient now use invoke_wrapper_raw under the hood.

@nerfZael nerfZael changed the base branch from main to nerfzael-subinvoke-resolution-context May 15, 2023 20:00
@nerfZael nerfZael changed the title Resolution context refactor Removal of resolution context in wrapper.invoke May 16, 2023
@nerfZael nerfZael marked this pull request as ready for review May 16, 2023 19:44
@nerfZael nerfZael requested review from cbrzn and namesty May 16, 2023 19:44
@nerfZael nerfZael changed the base branch from nerfzael-subinvoke-resolution-context to main May 16, 2023 22:07
@nerfZael nerfZael merged commit cfcc921 into main May 17, 2023
@cbrzn cbrzn deleted the nerfzael-resolution-context-refactor branch May 31, 2023 18:30
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.

3 participants