Skip to content

Conversation

@djkoloski
Copy link
Contributor

This bound is only necessary when calling methods on SendStream because we don't use any associated types of B's Buf impl in the type. Relaxing this bound allows us to construct SendStreams with non-Buf B (though we can't do anything with them).

@djkoloski
Copy link
Contributor Author

#616 should fix the nightly CI.

@seanmonstar
Copy link
Member

(though we can't do anything with them).

Is this beneficial somehow? I'd love to understand the goal.

@djkoloski
Copy link
Contributor Author

Sorry, I didn't put enough context in here. hyperium/hyper#2830 contains some work to remove a transmute from hyper. Rather than write new Buf impls with unreachable!, I figured we could clean things up bit by relaxing the bounds in h2. That way we don't have to write uncallable Buf impls with impossible methods. It's not very important, but it does help with code quality downstream.

@djkoloski
Copy link
Contributor Author

Bumping this after #616 was merged.

Copy link
Member

@LucioFranco LucioFranco left a comment

Choose a reason for hiding this comment

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

This seems fine to me, I prefer to not have the bounds on the struct anyways. Also, I don't think this is breaking so I think we are fine to merge?

@djkoloski
Copy link
Contributor Author

Quick bump on this, if there are no objections I think this would be a good improvement. 🙂

@LucioFranco
Copy link
Member

Oh sorry, we can get this merged :D

@LucioFranco LucioFranco merged commit 756e252 into hyperium:master Sep 6, 2022
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