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.

Sign up for GitHub

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

Jump to bottom

Migrate to sbt-typelevel #875

Merged
merged 11 commits into from
Mar 15, 2022
Merged

Migrate to sbt-typelevel #875

merged 11 commits into from
Mar 15, 2022

Conversation

armanbilge
Copy link
Member

No description provided.

@armanbilge armanbilge mentioned this pull request Mar 2, 2022
Open
44 tasks
armanbilge
armanbilge commented Mar 2, 2022
View reviewed changes
core/shared/src/main/scala/org/scalacheck/util/Buildable.scala
Comment on lines -30 to +36
private[scalacheck] implicit def buildableSeq[T]: Buildable[T, Seq[T]] =
def buildableSeq[T]: Buildable[T, Seq[T]] =
new Buildable[T, Seq[T]] {
def builder: mutable.Builder[T, Seq[T]] =
Seq.newBuilder[T]
}

private[scalacheck] implicit def implicitBuildableSeq[T]: Buildable[T, Seq[T]] = buildableSeq
Copy link
Member Author

Choose a reason for hiding this comment

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

A minor thing, but lets us keep bincompat with a static forwarder without adding an exclusion.

The problem was introduced in #788 (see discussion there).

build.sbt
Comment on lines +53 to 65
ThisBuild / tlBaseVersion := "1.15"
ThisBuild / tlMimaPreviousVersions ++= Set(
// manually added because tags are not v-prefixed
"1.14.0",
"1.14.1",
"1.14.2",
"1.14.3",
"1.15.0",
"1.15.1",
"1.15.2",
"1.15.3",
"1.15.4",
)
Copy link
Member Author

Choose a reason for hiding this comment

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

This is annoying, but gets the job done.

Any objection to using early-semver for scalacheck going forward?

Copy link
Member

Choose a reason for hiding this comment

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

As an alternative to regular semver? What's the difference when we're past 0.x?

Copy link
Member Author

Choose a reason for hiding this comment

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

For some reason I thought scalacheck may be using PVP? Happy to be wrong.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, the release notes say 1.14.x is not backwards-compatible with anything before it.
https://github.com/typelevel/scalacheck/blob/main/CHANGELOG.markdown#1140-2018-04-22

Copy link
Member

Choose a reason for hiding this comment

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

Oh, yeah. The breaking change in a minor version was a driver for a major version bump in libraries as prominent as Cats. Binary compatibility weighs very heavily on this project, and we shouldn't do that again.

build.sbt

ThisBuild / crossScalaVersions := Seq(Scala3, Scala212, Scala213)
Copy link
Member Author

Choose a reason for hiding this comment

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

This PR restores the single Scala 3 build. Of course, we can't merge it today, but once the -Yscala-release flag arrives I think this should work fine.

Incidentally, you'll find that CI is running Scala 3 + Native jobs :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, we could dial this back to Scala 3.0.2 and merge this today.

@armanbilge
Copy link
Member Author

Woops, conflicts...

@armanbilge armanbilge marked this pull request as ready for review March 3, 2022 01:03
rossabaker
rossabaker requested changes Mar 7, 2022
View reviewed changes
Copy link
Member

@rossabaker rossabaker left a comment

Choose a reason for hiding this comment

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

Seems good to me. Requesting changes to make sure the Scala 3 gets rolled back to 3.0.2, or else we're deciding for the whole world.

build.sbt
Comment on lines +53 to 65
ThisBuild / tlBaseVersion := "1.15"
ThisBuild / tlMimaPreviousVersions ++= Set(
// manually added because tags are not v-prefixed
"1.14.0",
"1.14.1",
"1.14.2",
"1.14.3",
"1.15.0",
"1.15.1",
"1.15.2",
"1.15.3",
"1.15.4",
)
Copy link
Member

Choose a reason for hiding this comment

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

As an alternative to regular semver? What's the difference when we're past 0.x?

@rossabaker rossabaker mentioned this pull request Mar 7, 2022
Closed
@rossabaker
Copy link
Member

Does this supersede #860?

@armanbilge
Copy link
Member Author

Yes, supersedes #860 with apologies to @vasilmkd who put a lot of time/effort into it as well. Also closes #625.

I rolled back to 3.0.2 and removed the Native/Scala3 job. Should be good to go.

@rossabaker
Copy link
Member

I didn't understand we lose native support on Scala 3 with this rollback. So our choices are:

  1. Keep blocking native support on Scala 3
  2. Force an upgrade on all the JVM clients that have stuck to 3.0 (or make them pin scalacheck too)
  3. Complicate this further by publshing the JVM on 3.0 and Native on 3.1

?

@armanbilge
Copy link
Member Author

armanbilge commented Mar 7, 2022 •
edited
Loading

Yes, Scala Native requires 3.1. I think we should wait until the new -Yscala-release flag arrives in 3.1.2, at which point we can compile all platforms on Scala 3.1 without forcing any downstream to upgrade.

FWIW, scalameta/munit#477 is an important PR blocked by this one, and they are also waiting for the new flag.

@armanbilge
Copy link
Member Author

See also portable-scala/sbt-crossproject#130 for why I don't want to mess around with option (3).

@rossabaker
Copy link
Member

I suppose a fourth option is a prerelease as the next minor version that builds for Scala 3.1, with the assumption that Scala 3.1.2 will precede it.

As someone almost entirely on Scala 2 and not at all on Scala Native, easy for me to say, but I'm content to wait.

@armanbilge
Copy link
Member Author

I'd be happy to get this PR merged as-is, and then we can discuss the Native/3 situation :)

rossabaker
rossabaker approved these changes Mar 7, 2022
View reviewed changes
Copy link
Member

@rossabaker rossabaker left a comment

Choose a reason for hiding this comment

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

I've never been a primary maintainer, so I won't hit the green button, but if nobody objects in the next couple days, I will.

@armanbilge
Copy link
Member Author

Thanks!

Gah, conflicts again!

@rossabaker
Copy link
Member

Sorry, I just merged one that had been approved for months.

@armanbilge
Copy link
Member Author

No worries, I'm glad to see activity here :)

@vasilmkd vasilmkd mentioned this pull request Mar 7, 2022
Closed
@SethTisue
Copy link
Member

I've never been a primary maintainer

I'm not sure if any of us here are "primary maintainer"s anymore

@SethTisue SethTisue merged commit 6ee7f25 into typelevel:main Mar 15, 2022
@armanbilge armanbilge mentioned this pull request Mar 15, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rossabaker rossabaker rossabaker approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@armanbilge @rossabaker @SethTisue