Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix latest-by-scala-version badge summary to prefer releases #1330

Conversation

rtyley
Copy link
Contributor

@rtyley rtyley commented Jan 11, 2024

I just noticed that the versions shown in the Scaladex badges for content-api-firehose-client don't match - one is showing the stable release number (1.0.12 馃憤), and the other is showing a pre-release (less good):

  • The 'Latest version' badge is showing a good version: 1.0.12
    image

  • The 'JVM badge': 1.0.13-PREVIEW.rt-dbremove-travis-file.2024-01-10T1738.6e259255
    image

image

The JVM badge shows a latest-by-scala-version summary I introduced with #660, but it looks like I missed out logic which is present in the other badge - preferring to show releases, and only showing pre-releases if they are the only available version!

This PR fixes the behaviour on the latest-by-scala-version badge to match the behaviour of the other badge - it does this with a new PreferReleases ordering for SemanticVersions. I did have a look to see if I could refactor the logic used by the first badge to also make use of this ordering, but it would have required a bigger refactoring:

val (releaseArtifacts, nonReleaseArtifacts) = artifacts.partition(_.version.isRelease)
artifactSelection
.defaultArtifact(releaseArtifacts, project)
.orElse(artifactSelection.defaultArtifact(nonReleaseArtifacts, project))

@rtyley rtyley force-pushed the fix-summary-of-latest-versions-badge-to-prefer-releases branch 6 times, most recently from d185480 to 9c07f4f Compare January 13, 2024 10:33
Comment on lines -25 to -35
it("should provide a concise summary of latest versions") {
Badges.summaryOfLatestVersions(
Map(
`2.11` -> Seq(`7.0.0`, `7.1.0`),
`2.12` -> Seq(`7.0.0`, `7.1.0`, `7.2.0`),
`2.13` -> Seq(`7.0.0`, `7.1.0`, `7.2.0`, `7.3.0`),
`3` -> Seq(`7.2.0`, `7.3.0`)
)
) shouldBe "7.3.0 (Scala 3.x, 2.13), 7.2.0 (Scala 2.12), 7.1.0 (Scala 2.11)"
}

Copy link
Contributor Author

@rtyley rtyley Jan 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To take advantage of this being a simple unit test, I've moved it further down the file (along with a couple of other new unit tests) into BadgesUnitTests - this doesn't extend ControllerBaseSuite and so can be quickly run in IntelliJ, etc, without having to start docker.

@rtyley rtyley changed the title Fix summary of latest versions badge to prefer releases Fix latest-by-scala-version badge summary to prefer releases Jan 13, 2024
@rtyley rtyley marked this pull request as ready for review January 13, 2024 11:49
@rtyley rtyley force-pushed the fix-summary-of-latest-versions-badge-to-prefer-releases branch from 9c07f4f to 8b27462 Compare January 13, 2024 12:14
Currently, for the `content-api-firehose-client`, the Scaladex badges page
is showing some contrasting values:

https://index.scala-lang.org/guardian/content-api-firehose-client/badges

* The 'Latest version' badge is showing a good version:
  1.0.12
* The 'JVM badge' (which shows a summary of latest versions by supported
  Scala version, introduced with scalacenter#660)
  is showing an undesireable pre-release version:
  1.0.13-PREVIEW.rt-dbremove-travis-file.2024-01-10T1738.6e259255

You can see both values on the versions page, but only if you tick the
'Show pre-release versions' button:

* https://index.scala-lang.org/guardian/content-api-firehose-client/artifacts/content-api-firehose-client
* https://index.scala-lang.org/guardian/content-api-firehose-client/artifacts/content-api-firehose-client?pre-releases=true#

This PR fixes the behaviour on the latest-by-scala-version badge to match the behaviour
of the other badge - it does this with a new PreferReleases ordering for SemanticVersions.
@rtyley rtyley force-pushed the fix-summary-of-latest-versions-badge-to-prefer-releases branch from 8b27462 to d16cc46 Compare January 13, 2024 12:27
Copy link
Member

@adpi2 adpi2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution!

@adpi2 adpi2 merged commit a7a4fa0 into scalacenter:main Jan 17, 2024
3 checks passed
@rtyley
Copy link
Contributor Author

rtyley commented Jan 17, 2024

Thanks! I can confirm that the badges on this sample project now show the correct release version:

https://index.scala-lang.org/rtyley/sample-project-using-gha-scala-library-release-workflow/badges

image

...even though there are 'higher' number preview releases:

https://index.scala-lang.org/rtyley/sample-project-using-gha-scala-library-release-workflow/artifacts/core?pre-releases=true#

image

rtyley added a commit to guardian/pan-domain-authentication that referenced this pull request Apr 26, 2024
The Maven badges we were using don't understand about pre-release version numbers, thankfully the Scaladex badges do!

See also:

* guardian/gha-scala-library-release-workflow#19 (review)
* scalacenter/scaladex#1330
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants