-
Notifications
You must be signed in to change notification settings - Fork 21
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
Adopt sbt-circe-org #361
Adopt sbt-circe-org #361
Conversation
ScalaCheck had the same issue with old tags, here's a workaround for MiMa. |
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.
Besides the mima version thing mentioned above, LGTM AFAICT 👍 thank you so much!
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.
Other than the mima fix that @armanbilge mentioned this looks great! Thank you so much
Co-authored-by: Arman Bilge <[email protected]>
config/src/main/scala-2.12/main/scala/io.circe.config/compat/package.scala
Outdated
Show resolved
Hide resolved
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.
Thanks again!
Sorry @zmccoy for dismissing your last review, but I noticed that my changes removed coverage reporting from CI. I thought that my last commit would bring it back, but the upload of the coverage report failed with:
Update: This needs to be fixed in sbt-circe-org: https://github.com/circe/sbt-circe-org/blob/d5ffac69ce6cd424388dc7267ad00671b6982cd5/core/src/main/scala/io/circe/sbt/CirceOrgPlugin.scala#L92-L94 |
The ${{matrix.java}} flag prevents the coverage report from being uploaded if the value of that variable contains an `@`, see circe/circe-config#361 (comment)
That variable prevents the coverage report from being uploaded if it contains an `@`, see: circe/circe-config#361 (comment)
Coverage reporting should be fixed with the next sbt-circe-org version but I don't think this is a blocker for this PR. |
This is similar to what @armanbilge did in circe/circe-fs2#394 so that releases can be done via CI.
Note that circe-config's version tags currently don't start with a
v
but sbt-typelevel mandates that. So after this is merged, new tags with thev
prefix that point to the old tags should be added and pushed:Without that,
version
andmimaPreviousArtifacts
for example won't be set correctly.