Add tlLatestVersion, tlLatestPreReleaseVersion to versioning plugin
#333
Conversation
kernel/src/main/scala/org/typelevel/sbt/TypelevelKernelPlugin.scala
Outdated
Show resolved
Hide resolved
versioning/src/main/scala/org/typelevel/sbt/TypelevelVersioningPlugin.scala
Outdated
Show resolved
Hide resolved
| currentVersion.forall(v.copy(prerelease = None) <= _) | ||
| } | ||
| } | ||
| lazy val currentPreRelease = currentPreReleaseImpl |
There was a problem hiding this comment.
We should define these as proper settingKeys with descriptions, that are populated below based on the implementations.
There was a problem hiding this comment.
Also, we should be mindful about naming. tl-prefixed to avoid collisions with other plugins.
kernel/src/main/scala/org/typelevel/sbt/TypelevelKernelPlugin.scala
Outdated
Show resolved
Hide resolved
|
I removed the exposed |
site/src/main/scala/org/typelevel/sbt/TypelevelSitePlugin.scala
Outdated
Show resolved
Hide resolved
| lazy val tlCurrentRelease = | ||
| settingKey[Option[String]]( | ||
| "The latest stable released version of your project, e.g. 0.2.0, 3.5.1. If applicable, this will be the version currently being released.") | ||
|
|
||
| lazy val tlCurrentPreRelease = settingKey[Option[String]]( | ||
| "The latest pre-release (e.g. milestone, release candidate) of your project. If applicable, this will be the version currently being released.") |
There was a problem hiding this comment.
Thanks, these are great descriptions! Bikeshed time, is "current release" and "current pre-release" the best terminology for these? Maybe "latest" is more appropriate? And "version" instead of "release"?
I guest that gets us to something like tlLatestStableVersion and tlLatestPreReleaseVersion maybe 🤔 bit of a mouthful.
There was a problem hiding this comment.
I have absolutely no opinion on this but happy to change it if you like 😃
There was a problem hiding this comment.
Not even a smidgeon of an opinion? 😂 I hate deciding these things. I can commit whatever I decide before I merge.
There was a problem hiding this comment.
Maybe I have a slight preference for tlLatestStableRelease and tlLatestPreRelease - explicit but reasonably concise.
There was a problem hiding this comment.
On the other hand, currentRelease isn't actually guaranteed to be a stable release:
if the latest release is a pre-release (e.g., M or RC)
and there are no stable releases it is bincompatible with,
then for all effective purposes it is the current release
There was a problem hiding this comment.
Can I leave this with you to commit the final wording and naming? Happy to discuss ideas of course
There was a problem hiding this comment.
Yup, I can do that. Thanks for all your help, nice to bounce ideas! Much appreciated.
There was a problem hiding this comment.
I think the first one is maybe something like:
The latest stable version of your project in the
tlBaseVersionseries, with the latest pre-release in this series used as a fallback if there is no stable release yet.
Ah yeah sounds good, I only just saw this now.
There was a problem hiding this comment.
The latest stable version of your project in the
tlBaseVersionseries, with the latest pre-release in this series used as a fallback if there is no stable release yet.
Actually I don't think that's correct? tlBaseVersion isn't used in the calculation, and this won't be the behaviour if (say) tlBaseVersion is 1.4 but there's a stable 1.3.0 release and a 1.4.0-M1 release, right?
There was a problem hiding this comment.
You are right, it's still not 100% correct.
tlBaseVersionisn't used in the calculation
It is, albeit indirectly. version is used in the calculation, and tlBaseVersion is used to calculate version.
tlBaseVersionis 1.4 but there's a stable 1.3.0 release and a 1.4.0-M1 release, right?
You are right about this. It uses the binary series and not the x.y series.
Co-authored-by: Ben Plommer <ben.plommer@gmail.com>
| private[sbt] lazy val currentRelease: Def.Initialize[Option[String]] = Def.setting { | ||
| // some tricky logic here ... | ||
| // if the latest release is a pre-release (e.g., M or RC) | ||
| // and there are no stable releases it is bincompatible with, | ||
| // then for all effective purposes it is the current release | ||
|
|
||
| val release = previousReleases.value match { | ||
| case head :: tail if head.isPrerelease => | ||
| tail | ||
| .filterNot(_.isPrerelease) | ||
| .find(head.copy(prerelease = None).mustBeBinCompatWith(_)) | ||
| .orElse(Some(head)) | ||
| case releases => releases.headOption | ||
| } | ||
|
|
||
| release.map(_.toString) | ||
| } |
There was a problem hiding this comment.
Shoot, still not quite right 😂 I thought this was using version in here, but you are right that it's not. Sorry, I was confusing with the change I made in #282 which does use version.
So as it's currently defined, even if you've bumped your base version, if you haven't actually tagged anything with that new base version then it will still use whatever the latest tag is under the old base version. I guess this makes sense. But, now to explain it 🤔 tl;dr we do some blackbox heuristics to get you a sensible number, take it or leave it 😛
| lazy val tlLatestVersion = settingKey[Option[String]]( | ||
| "The latest tagged version on this branch. Priority is given to the latest stable version, but if you have tagged a binary-breaking prelease version (such as a milestone or release candidate), that will be selected instead. If applicable, this will be the current tagged version.") | ||
|
|
||
| lazy val tlLatestPreReleaseVersion = settingKey[Option[String]]( | ||
| "The latest tagged version on this branch, including milestones and release candidates. If applicable, this will be the current tagged version.") |
There was a problem hiding this comment.
ok, @bplommer, what do you think? phew 😓
There was a problem hiding this comment.
I'm happy with that 👍
There was a problem hiding this comment.
On second thought, what do you think about tlLatestVersionOnBranch and tlLatestPreReleaseVersionOnBranch? It's a bit wordy, but for me it's really counterintuitive that (for example) these would be 0.22 versions when running on the http4s 0.22.x branch, so they're not really the latest versions.
There was a problem hiding this comment.
but for me it's really counterintuitive that (for example) these would be 0.22 versions when running on the http4s 0.22.x branch
Ok yeah that's super fair. But that is quite a mouthful. What if we used the terminology "previous version" instead of "latest"? Then maybe it would be more clear it's relative to our current context. The problem is that it muddies the fact that this can be the current version if the current commit is tagged.
There was a problem hiding this comment.
I don't like "previous", for the reason you give - to me that explicitly rules out it being the current one. But I don't have any other suggestions at the moment 🤔
There was a problem hiding this comment.
I guess my question is, what are you doing with tlLatestVersion on the 0.22.x branch, where you want it to be an 0.23+ version?
I can't think of anything! But I can imagine wanting the "latest prerelease" version on the 0.23.x branch to be a 1.0 milestone, so I could populate the website's landing page accordingly.
There was a problem hiding this comment.
In that case, shouldn't the landing page be published from main instead of series/0.23? (This is how we do it in http4s).
There was a problem hiding this comment.
Hmm, I guess not necessarily. My proposal in #317 aims to reduce that complexity, so that you only publish the website from the stable branch. Blah again 😂
There was a problem hiding this comment.
Blah indeed. Let's just go with it as it is, I think it's good enoigh?
There was a problem hiding this comment.
Yolo. We might end up adding a couple more of these as the complaints roll in 😂 thank you so much for your work on this.
tlLatestVersion, tlLatestPreReleaseVersion to versioning plugin
Resolves #318