-
Notifications
You must be signed in to change notification settings - Fork 52
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
Encapsulate svc preprocessing #197
Encapsulate svc preprocessing #197
Conversation
…lChannelProcessor (prev StaticVirtualChannel) and ChunkProcessor
Coverage Report 🤖 ⚙️Past: New: Diff: +1.50% [this comment will be updated automatically] |
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.
Looks reasonable to me! I left a few comments.
@pacmancoder
Since this is targeting your branch, I think it’s best if you merge it yourself once you reviewed it (make sure to squash)
EDIT: I see in the CI that something is not compiling
/// Usually returned by the channel-specific methods. | ||
pub struct SvcRequest<C> { | ||
pub struct SvcMessagesForProcessor<P: StaticVirtualChannelProcessor> { |
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 think For
could be omitted: SvcProcessorMessage
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.
LGTM
This branch also includes refactoring changes by @ibeckermayer from #196 and #197
This branch also includes refactoring changes by @ibeckermayer from #196 and #197 Co-authored-by: Isaiah Becker-Mayer <isaiah@goteleport.com>
Includes refactoring changes by @ibeckermayer from #196 and #197 Co-authored-by: Isaiah Becker-Mayer <isaiah@goteleport.com>
No description provided.