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

Migrate content length header & co #4547

Merged

Conversation

phongngtuan
Copy link
Contributor

Following up the session led by @rossabaker, partially address #4535

  • Add syntax for Header & Select
  • Add implicit from Select[A] to Renderer[A]
  • Migrate some of headers

Only the first commit is more or less interesting where the machinery are introduced. Really appreciate if you could take a look before I stray too far from the path. In particular

  • SimpleHeadersSpec used to take key + value but now only works on value.
  • HeadersSpec

todo:

  • migrate the rest of the headers
  • revisit the headers classes to clean up unused method/val (key, name,...) that was from inheritance. I left those as is since I don't know if anyone is using those

@phongngtuan phongngtuan force-pushed the migrate_content_length_header branch from 95b57c9 to 4023d5d Compare March 1, 2021 16:54
@@ -93,6 +96,11 @@ object Renderer {
override def render(writer: Writer, values: Set[T]): writer.type =
writer.addSet(values)
}

implicit def headerSelectRenderer[A](implicit select: v2.Header.Select[A]): Renderer[A] =
Copy link
Member

Choose a reason for hiding this comment

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

I think this is the only case where an http4s model type appears in this file. If it works putting this on Select instead of Renderer, I think that would be preferrable. I also just want to get this all compiling again, so I'm content to revisit this later.

@rossabaker rossabaker merged commit 9d879c3 into http4s:series/0.22 Mar 2, 2021
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

2 participants