slf4j depends on core jvm, not core js#644
Conversation
|
I think this would require a 2.4 release? cc @danicheg if you can weigh in please, I got rusty with releases for the past year 😅 |
|
The issue causes log4cats-slf4j not to declare its dependency on log4cats-core, and to declare an unneeded dependency on log4cats-core_sjs1 The chance should result in only a different POM. That means in terms of binary compatibility, it's backwards and forwards compatible, and doesn't require a minor bump. |
|
|
||
| lazy val slf4j = project | ||
| .settings(commonSettings) | ||
| .dependsOn(core.js) |
There was a problem hiding this comment.
Actually, that was discussed when @armanbilge was adding it #592 (review).
There was a problem hiding this comment.
Oops. This is 💯 my mistake and I really must not have been paying attention since your comment point it out 😳 I think I was trying to change coreJVM to core.jvm but I wrote core.js ... sorry everyone.
There was a problem hiding this comment.
In a Just world, either the build system or the compiler should have given you a fatal warning for this. This is a way too easy mistake to make.
There was a problem hiding this comment.
What really baffles me is that the tests run and pass using the JS artifact ...
There was a problem hiding this comment.
@armanbilge It's my fault too. I wasn't insistent and thought it's no matter what we'd use in that place :)
|
I agree, this can go into a 2.3.1. |
|
@martijnhoekstra Could you please rebase your branch? I've fixed the error that caused the failing CI in this PR. |
should close typelevel#643
|
Yes, rebased |
danicheg
left a comment
danicheg
left a comment
There was a problem hiding this comment.
Thank you so much for fixing this!
4 participants
should close #643