-
Notifications
You must be signed in to change notification settings - Fork 53
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
Added a http4s0.23 version to help with cats effect 3 upgrades. #220
Conversation
need to run |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but I'm no pact expert
how desperately do you need? I would rather not do a full release with an RC version |
I don't absolutely need it, but I've been working on the upgrade on one of oasvc services for a week & this is the last piece I need. The upgrade could just stay on a branch until we have a full release of that http4s stream. Not sure how long that'll be though. But on the other hand, not much is going to need to be done to that service either, so the branch is unlikely to go stale. |
It looks like the only reason https23 is at RC1 still is because they're waiting for circe 0.14, which is already released. Hopefully they'll do a full release soon.... let's wait & see. |
yep, lets give the http4s guys a little bit to catch up with our lightning fast development 😉 |
I've converted this to draft. It's not quite right anyway. |
class PactStubber extends IPactStubber { | ||
|
||
private var instance: Option[() => Future[Unit]] = None | ||
private var _port: Option[Int] = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The way this becomes a Some leads to a race condition. See line 57 of this file & line 61ish of ScalaPactMock. I don't know why it isn't a problem in the CE2 version.
I've run |
@dantb go ahead! |
No description provided.