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

Adding Header.Select fromSafe, toRaw multiple, and Aux #4787

Merged
merged 7 commits into from May 24, 2021

Conversation

zarthross
Copy link
Member

@zarthross zarthross commented Apr 25, 2021

Some enhancements needed to help enable Rho support.

See: http4s/rho#501 for some usage example.

I could use feedback on these, once I get buy in I'll happily add/fixup the unit tests!

Resolves #4706

@rossabaker
Copy link
Member

We should probably target 0.22 with this: that's where the new header model debuts. It shouldn't be hard to merge to main, since it's pure work.

@rossabaker rossabaker added breaking Change breaks compatibility (binary or source) module:core labels Apr 26, 2021
@rossabaker rossabaker added this to the 0.22 milestone Apr 26, 2021
@zarthross zarthross changed the base branch from main to series/0.22 April 26, 2021 22:49
@zarthross
Copy link
Member Author

We should probably target 0.22 with this: that's where the new header model debuts. It shouldn't be hard to merge to main, since it's pure work.

Done.

@rossabaker
Copy link
Member

Haven't forgotten this. 0.21 sucked up all my time, but this is absolutely necessary before 0.22 so we don't leave rho hanging. Extra opinions welcome.

@rossabaker
Copy link
Member

So this is still in draft. We want to release 0.22.0, but are blocked by a couple issues and circe. I also want to unblock Rho. We could merge this as is and clean it up before the final release, or we could let rho work off snapshots/source dependencies and clean up before the final release. Thoughts?

@zarthross
Copy link
Member Author

I think I can make some time this weekend for the suggestions here, and hopefully any associated testing.

@rossabaker
Copy link
Member

That sounds great. I have a stupid busy weekend, so if we can get things greened up, I may do a milestone today. But I will commit to do another release as soon as this is ready, because we don't want to leave Rho hanging.

@zarthross
Copy link
Member Author

Just 2 questions that need answering on the above issues, and I'd be ready to move this out of Draft.

@zarthross zarthross marked this pull request as ready for review May 23, 2021 22:14
rossabaker
rossabaker previously approved these changes May 23, 2021
Copy link
Member

@rossabaker rossabaker left a comment

Choose a reason for hiding this comment

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

I think this is sufficient, and we can do an RC with it when circe-0.14 is ready and make sure Rho has a path forward.

core/src/main/scala/org/http4s/syntax/HeaderSyntax.scala Outdated Show resolved Hide resolved
@rossabaker rossabaker dismissed their stale review May 23, 2021 22:26

We broke a test

@rossabaker
Copy link
Member

Looks like we also broke this:

 ==> X org.http4s.client.middleware.LoggerSuite.RequestLogger should not affect a Post  0.008s org.http4s.ParseFailure: Invalid Content-Type header: Content-Type header doesn't support media ranges

@zarthross
Copy link
Member Author

Looks like we also broke this:

 ==> X org.http4s.client.middleware.LoggerSuite.RequestLogger should not affect a Post  0.008s org.http4s.ParseFailure: Invalid Content-Type header: Content-Type header doesn't support media ranges

I can take a look.

@rossabaker rossabaker merged commit 7bb4f80 into http4s:series/0.22 May 24, 2021
@zarthross zarthross deleted the Header-Select-Enhancements branch October 29, 2021 21:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Change breaks compatibility (binary or source) module:core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants