✨ Select highest tag when there are sibling tags

[jprudent]

This fixes issue #238

[jprudent]

Oh I didn't notice tests were not passing on CI. I don't have time today I think. I will definitely try to fix those.

[SethTisue]

I had to approve CI to run. I'll see any further updates sooner now that I'm watching the PR.

[jprudent]

I run the tests locally. They are passing.
Thanks for your review!

[SethTisue]

I'll take a look later this week.

[SethTisue]

(sorry for the slower-than-promised response time)

Well, this looks reasonable to me, though I don't know the code in this repo — my involvement here is largely administrative. So I'd be inclined to optimistically merge, as it could always be refined later.

But I have one question for @dwijnand and @eed3si9n — is there prior art for the version number comparison logic, something we could be using instead of adding or own custom code...? (I imagine this wheel has been reinvented many times in many programming languages...) Something in sbt itself perhaps?

[eed3si9n]

The exact detail of what's considered high or low actually depends on the impl detail of Ivy or Coursier for somethings like 1.0-A1 vs 1.0-RC1, iirc but sbt implements https://www.scala-sbt.org/1.x/api/sbt/librarymanagement/SemanticSelector.html, which lets you create a semantic versioning selector a la npm version expression, which can say greater than based on SemVer logic, which is usually good enough.

[SethTisue]

@jprudent want to take a stab at using the API Eugene suggests?

[jprudent]

sure! let's change the status of PR to changes required, I'll have a look soon

[jprudent]

Hello fellows, I need a bit of advice.

This project has 2 modules:

• dynver which contains the "pure" logic and independant of SBT
• sbtdynver which contains the nuts and bolts of an Sbt plugin. It depends of dynver.

The class SemanticSelector is provided by librarymanagement-core which is an SBT artifact, and not available in dynver module. To use it I need to introduce a dependency or merge both modules. What would you do ?

[dwijnand]

Add the dependency to the core.

[jprudent]

Hello,

I added the dependency as provided. I also added a few tests.

sbt test ✅

Commits can be squashed.

[SethTisue]

I added the dependency as provided

What's the reasoning there?

Note that CI is failing.

[jprudent]

Hello Seth, thanks again for your time and feedback.

I didn't notice that the project was cross compiled. The CI does a sbt +test that runs tests for all scala versions, I didn't know.

I just changed the dependency to pull the correct artifact depending of scala version. Note that scala 3 artifacts are still alpha in the repo

The theory behind the Provided was that since the plugin is running in SBT, it already has the library in its classloader when executed and doesn't need to hard depend on it.
But with the new definition of dependency in my last commit fce4d43, the dependency clashes with the one provided by SBT (tests failed in scripted). So I will reintroduce Provided. The danger with this kind of dependency is that the code is compiled against a given version of it, but at runtime the API may change or the Class can't be found. Since I only use a very small subset of it that looks quite stable (VersionSelector and SemanticSelector) we can expect that will never happen.

I will set the dep as Provided and I will write a scripted test (that I discovered in the process...) in a future commit of this PR.

Thanks again and sorry for all that churn.