Skip to content
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

Drop AsHandler method from Mux #77

Merged
merged 1 commit into from
Sep 27, 2023
Merged

Drop AsHandler method from Mux #77

merged 1 commit into from
Sep 27, 2023

Conversation

emcfarlane
Copy link
Collaborator

No description provided.

@@ -110,30 +112,30 @@ type JSONCodec struct {
UnmarshalOptions protojson.UnmarshalOptions
}

var _ StableCodec = (*JSONCodec)(nil)
var _ RESTCodec = (*JSONCodec)(nil)
var _ StableCodec = JSONCodec{}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks unrelated and not sure we need to do it, but not blocking for now since we're reviewing the Codec interfaces separately.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It removes having to have the cache to convert codecs from a map[string] func(TypeResolver) Codec to map[codecKey]Codec where the codecKey is a type of struct{string, TypeResolver} which means we can drop the constructor for the handler.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a little less efficient, could move the cache to the methodConfig but if we are changing the codec layout easier to call the factory func.

Base automatically changed from ed/names to main September 27, 2023 15:00
@emcfarlane emcfarlane merged commit a42128f into main Sep 27, 2023
4 checks passed
@emcfarlane emcfarlane deleted the ed/muxserve branch September 27, 2023 15:09
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