-
-
Notifications
You must be signed in to change notification settings - Fork 354
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
Support version scheme in pom #1629
Conversation
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.
That was quick, thank you!
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 for this contribution. Can you tell, if coursier actually supports this info.versionScheme
property, so that we also can benefit from it, or is this only used by sbt to filter coursier version resolutions by impact?
This PR effectively adds some property to the pom.xml
, so I think the better implementation would be to add generic property support to mill.scalallib.publish.Pom
, and then just derive the correct property from the selected version scheme. Also adding a properties
or pomProperties
setting to PublishModule
would be nice, but can also be done later in a separate PR. I don't know, if the ivy file also support properties, but if, we should add the versionScheme property there too.
As far as I know, the logic is implemented in sbt/librarymanagement: https://github.com/sbt/librarymanagement/blob/develop/core/src/main/scala/sbt/librarymanagement/EvictionError.scala Here is Coursier documentation about versions handling: https://get-coursier.io/docs/other-version-handling I believe it would make sense to move that logic upstream to coursier. Maybe @alexarchambault or @eed3si9n have an opinion?
I agree. |
@julienrf Thanks for the pointers. I think properties are in general useful for interoperability with downstream consumers. |
This PR needs to be rebased on latest main branch. The |
7adf64a
to
e24a0b3
Compare
Rebased the PR and applied the suggested change. Defining a override def versionScheme = VersionScheme.EarlySemver Instead of: override def versionScheme = Some(VersionScheme.EarlySemver) I took the idea from Anyway, @lefou let me know which one you prefer. |
I think the use of an |
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. Thank you very much!
Sbt recently added the possibility to state the versioning scheme in poms to allow precise binary compatibility checks.
https://scala-lang.org/blog/2021/02/16/preventing-version-conflicts-with-versionscheme.html
This PR implements some basic support to add the versionScheme to
PublishModule
s.All the naming and packages are arbitrary.
Suggestions accepted :) (as usual)
cc. @julienrf