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

util: impl Clone for AndThen, MapRequest and MapErr layers #525

Merged
merged 3 commits into from
Jan 12, 2021

Conversation

hawkw
Copy link
Member

@hawkw hawkw commented Jan 12, 2021

The Layer types for AndThen, MapRequest, and MapErr middleware
were missing Clone impls, which the MapResponse, MapResult, and
Then middleware's layer types do have. These layers should all be
Clone when the function is Clone.

I've added derive(Clone) to all these layer types.

The `Layer` types for `AndThen`, `MapRequest`, and `MapErr` middleware
were missing `Clone` impls, which the `MapResponse`, `MapResult`, and
`Then` middleware's layer types *do* have. These layers should all be
`Clone` when the function is `Clone`.

I've added `derive(Clone)` to all these layer types.
@hawkw hawkw requested a review from a team January 12, 2021 18:27
@hawkw hawkw self-assigned this Jan 12, 2021
Copy link
Collaborator

@olix0r olix0r left a comment

Choose a reason for hiding this comment

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

You could probably derive Copy as well

@hawkw
Copy link
Member Author

hawkw commented Jan 12, 2021

You could probably derive Copy as well

yes, although it's a potential compatibility issue if we ever add additional fields that are not trivially Copy. i don't think that's too likely, but i punted on copy for now.

@hawkw hawkw merged commit b8927bc into master Jan 12, 2021
@hawkw hawkw deleted the eliza/missing-clones branch January 12, 2021 21:23
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.

None yet

3 participants