search

kamon-io / kamon-http4s

Kamon Integration for http4s
Apache License 2.0
53 stars 29 forks source link

Upgrade http4s to 0.20.0-M4 #18

Closed kubukoz closed 5 years ago

kubukoz commented 5 years ago

Oops, just realised it's 0.19-M3 now :D updating soon

dpsoft commented 5 years ago

@kubukoz thanks for proposing this PR, looks good to me. @garraspin @Widar91 @ptrlaszlo any comments?

kubukoz commented 5 years ago

Hey @dpsoft, I was going to make some more changes, for now only the client part seemed worthy of looking at :) I'll probably change some more things in the server part to use more of Resource, etc.

saraiva132 commented 5 years ago

You can already use http4s 0.19.0 Any thoughts when you will have time @kubukoz, I was thinking on doing this myself but did not find the time and very fluent with Resource/Bracket and then I saw this PR and it looks good !

kubukoz commented 5 years ago

0.19.0 is a mistag, equivalent to 0.19.0-M4 :)

On Sun, Oct 21, 2018, 12:59 Rafael Saraiva Figueiredo < notifications@github.com> wrote:

You can already use http4s 0.19.0 :D

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/kamon-io/kamon-http4s/pull/18#issuecomment-431659227, or mute the thread https://github.com/notifications/unsubscribe-auth/AA2npMdfPowkfV1F2O0Xt2SZVLgSCKOOks5unFOggaJpZM4XBGz0 .

saraiva132 commented 5 years ago

ahhhh, I noticed that something was weird with that release since they also release the M4. I assumed the M4 was the mistag since version is currently set at 0.19.1-SNAPSHOT in master. Good of you to clarify :)

kubukoz commented 5 years ago

Yeah, 0.19.0 was released by mistake and 0.19.1 will most likely be binary breaking.

kubukoz commented 5 years ago

Aaaand there won't be a stable 0.19, now we're on 0.20 milestones.

ptrlaszlo commented 5 years ago

Any plans for merging this branch? (using v0.20.0-M3 maybe?)

dpsoft commented 5 years ago

@kubukoz we can review this PR or is there something left to do?

kubukoz commented 5 years ago

Sorry, I'm quite busy this week, but I'll try to get this ready sometime next week (hopefully in time for a http4s release). The latest changes I made broke some things (hence the build failure), and on top of that I was unable to get sensible traces when testing this out with a simple http4s app. Which I now realize has been an issue even before this PR, so there's more investigation to be done.

If anyone wants to take over this, feel free to. As I said, I probably won't get back to this until late next week :(

notthatbreezy commented 5 years ago

I think the CI failure was related to the old kamon-sbt-umbrella reference which was fixed in #19. Rebasing this on master would fix the error I think (at least it did the trick for me).

I'm happy to update this if that would help. Would opening a separate PR work? Not sure what you all would prefer.

kubukoz commented 5 years ago

Managed to restart work on this, tried to do it without as many changes as before.

Notable things:

kubukoz commented 5 years ago

Please review @ivantopo @dpsoft :)

dpsoft commented 5 years ago

@kubukoz awesome man!!, please remember to sign the CLA to merge this PR :) Thanks a lot

kubukoz commented 5 years ago

Done :)

kubukoz commented 5 years ago

Hey @dpsoft, can you make a release with this?

dpsoft commented 5 years ago

@kubukoz done : https://github.com/kamon-io/kamon-http4s/releases/tag/v1.0.11