Issue 756/Rename GetExtension to GetInputExtension and create GetOutputExtension #759
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
GlenDC
approved these changes
Dec 9, 2025
Member
GlenDC
left a comment
GlenDC
left a comment
There was a problem hiding this comment.
Looks good enough. You seem to have gotten it well :)
Thank you!
Member
|
@M-Kusumgar if the CI passes I'll merge it in. Feel free if you have the motivation to open a 2nd PR to do the same for |
Member
|
@M-Kusumgar seems the basic checks fail. just qqin your local environment, fix if something has to be fixed and commit the changes made by you and/or tooling. |
Contributor
Author
|
was just some formatting |
GlenDC
pushed a commit
that referenced
this pull request
Dec 9, 2025
…xtension (#761) Couple of thoughts I had while doing this one: * I am not sure if my example at the top of `add_extension` with the `ResponseCounter` is a good one, happy to change it/remove it if we don't need an example * The `serve` function for `AddInputExtension` used to return a `Future<>` but for `AddOutputExtension` I needed to await to get the response, so I made both of them consistent and converted them into `async fn`s. This is also consistent with `GetInputExtension` and `GetOutputExtension`. I am not super familiar with async rust (let alone rust) so I am not 100% sure about the implications of this change but I have done a bit of looking into it so `async fn` is the same as returning a `Future<>` from a function that wraps the original function body in an `async move`. The things im not sure about are: * `'_` - im not sure we need this, presumably we want the returned value to live as long as the args, is that the default for async functions? * `Send` - not sure if we need to enforce it on the return here, `Input` has the trait enforced and its the only one we take a mutable reference to Relates to #759
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #756
I also think I need to create a
GetOutputExtensionLayerfor theGetOutputExtension. Is this what you had in mind? I basically added a copy ofservebut instead it passes the cloned value of the extension from the output to the callback passed in.