search

hyperium / http

Rust HTTP types
Apache License 2.0
1.14k stars 284 forks source link

Merge #428 and #499 #540

Closed jeddenlea closed 2 years ago

jeddenlea commented 2 years ago

For your consideration. I'm not sure what the ideal way to collapse these commits together would be to match the clean, ordered scheme this repo is otherwise doing. But, if we were to apply #428 atop #499, I think it would look like this.

LucioFranco commented 2 years ago

Hi @jeddenlea I think due to the nature of those two PRs I think it probably makes sense to review them individually and have separate commits. So we should try to revive those again even though there will be some conflicts.

jeddenlea commented 2 years ago

@LucioFranco certainly! I won't be upset if this PR is abandoned, I just wanted to work out what a merge would look like because it would be great for both to be included in the next release, and there are lots of conflicts. This can be used as a reference if that's helpful. I did update it to include the small tweak I had to make on #499 for final approval.

seanmonstar commented 2 years ago

Looks like we got conflicts from merging first your other PR. I've read through the code and changes, everything is super clear, thank you! Want to try to rebase and we'll get a release out with this?

seanmonstar commented 2 years ago

Well I tried resolving the conflicts with GitHub's help, and it failed in one tiny way. So I've now fixed it up locally, and ended up squashing it into a single commit since the diff was quite small now that #499 was merged before. The new PR is in #555.

seanmonstar commented 2 years ago

Merged in #555.