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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(ci): migrate to sbt-ci-release #1962

Merged
merged 4 commits into from
Dec 27, 2022
Merged

refactor(ci): migrate to sbt-ci-release #1962

merged 4 commits into from
Dec 27, 2022

Conversation

ckipp01
Copy link
Member

@ckipp01 ckipp01 commented Dec 22, 2022

This pr changes the way we do releases and migrates us away from the
forked sbt-release-early that's no longer maintained and is also
bringing in some old versions of scala-xml which will be problematic
when we try to update our sbt stuff.

Plus this simplifies the release a little bit. The one difference is
that we'll now be publishing actual -SNAPSHOT into sonatype snapshots.

Refs: #1680

@ckipp01 ckipp01 requested review from tgodzik and adpi2 December 22, 2022 11:42
Copy link
Member Author

@ckipp01 ckipp01 left a comment

Choose a reason for hiding this comment

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

One other thing I noticed is that we actually publish bloop-shared. However the other modules are using it via dependsOn and I don't think anything else actually uses this. Do we need to keep publishing it separately?

.github/workflows/ci.yml Show resolved Hide resolved
Comment on lines -286 to -288
mkdir "$HOME/.ssh"
mkdir "$HOME/.sbt"
mkdir "$HOME/.sbt/gpg"
Copy link
Member Author

Choose a reason for hiding this comment

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

It's seems that this and also the couple things that I removed down below are all covered by sbt-ci-release, so there shouldn't be any reason to manually do this anymore. We won't really be able to tell until we just try.

Copy link

@julienrf julienrf left a comment

Choose a reason for hiding this comment

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

Thank you @ckipp01, this simplifies the build a lot!

Could you please check that versionScheme is defined?

.github/workflows/ci.yml Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
build.sbt Show resolved Hide resolved
@ckipp01
Copy link
Member Author

ckipp01 commented Dec 22, 2022

Could you please check that versionScheme is defined?

It's actually not. I'm assuming we want to early-semver for this, or did you have something else in mind?

@julienrf
Copy link

Could you please check that versionScheme is defined?

It's actually not. I'm assuming we want to early-semver for this, or did you have something else in mind?

early-semver would be perfect, but now that I start thinking about it I wonder if things are more complex than that. Especially the fact that we publish artifacts that contain shaded dependencies and others that don’t shade (probably those types of artifacts should have independent version numbers…).

In any case, if we claim that this project follows early-semver, then we should also set up checks to make sure we effectively conform to that claim. That means setting up MiMa or using sbt-version-policy.

I think that should be addressed separately.

@ckipp01
Copy link
Member Author

ckipp01 commented Dec 22, 2022

I think that should be addressed separately.

Yes, I agree. Regarding even the versioning of the shaded stuff, I don't even think we need to shade. I was talking to the other offline about this, created this discussion and also this pr to address / talk about it because the shading also brings a ton of complexity that is all to avoid hypothetical issues as far as I can tell. Seeing that so few tools use Bloop and we mostly know about them, I feel ok dropping that and then readdressing if we do indeed have an issue.

(publish / skip) := true,
publishLocalAllModules := {
BuildDefaults
.publishLocalAllModules(allProjectsToRelease)
Copy link
Member

Choose a reason for hiding this comment

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

It seems that allProjectsToRelease contained more project than allProjectReferences. In the diff there are all the shaded and the buildpress projects. Will they be released by ci-release or, is it okay not to release them?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah good catch, yes I am missing a couple that I didn't realize weren't in there. I knew the shaded stuff wasn't in there, but I was hoping to get some thoughts on #1961 before finishing up this pr 😆. Apart from that it looks like I'm also missing the build press stuff. Just to compare here's what a publishLocal will give you:

 bloop-backend_2.12
 bloop-frontend_2.12
 bloop-js-bridge-0-6_2.12
 bloop-js-bridge-1_2.12
 bloop-launcher-core_2.12
 bloop-launcher-core_2.13
 bloop-launcher-test_2.12
 bloop-native-bridge-0-4_2.12
 bloop-shared_2.12
 bloopgun-core_2.12
 bloopgun-core_2.13
 sbt-bloop
 sockets

vs what was in the publish list before:

  bloopShared, // TODO do we actually need to publish this???
  backend,
  frontend,
  sbtBloop,
  nativeBridge04,
  jsBridge06,
  jsBridge1,
  sockets,
  bloopgun,
  bloopgunShaded,
  bloopgun213,
  bloopgunShaded213,
  launcher,
  launcherShaded,
  launcher213,
  launcherShaded213,
  buildpressConfig,
  buildpress

I'll make sure the others get added, but it'll be nice to get some thoughts on the shaded stuff first, because ideally we wouldn't have to include those here as well.

Copy link
Member Author

@ckipp01 ckipp01 Dec 24, 2022

Choose a reason for hiding this comment

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

Alright now that the shaded stuff is gone, the new publish list will look like this:

bloop-backend_2.12
bloop-frontend_2.12
bloop-js-bridge-0-6_2.12
bloop-js-bridge-1_2.12
bloop-launcher-core_2.12
bloop-launcher-core_2.13
bloop-native-bridge-0-4_2.12
bloop-shared_2.12
bloopgun-core_2.12
bloopgun-core_2.13
buildpress_2.12
buildpressconfig_2.12
sbt-bloop
sockets

Copy link
Contributor

@tgodzik tgodzik left a comment

Choose a reason for hiding this comment

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

This looks awesome! Thanks @ckipp01 for looking into all those issues! 😍

This pr changes the way we do releases and migrates us away from the
forked sbt-release-early that's no longer maintained and is also
bringing in some old versions of scala-xml which will be problematic
when we try to update our sbt stuff.

Plus this simplifies the release a little bit. The one difference is
that we'll now be publishing actual `-SNAPSHOT` into sonatype snapshots.
This needed to be done after the work on moving to sbt-ci-release due to
scala-xml conflicts.

Supercedes #1925
Supercedes #1899
@ckipp01 ckipp01 force-pushed the sbt-ci-release branch 2 times, most recently from 60a75d9 to 77e00cf Compare December 24, 2022 10:27
@ckipp01 ckipp01 merged commit 2b4ee8a into main Dec 27, 2022
@ckipp01 ckipp01 deleted the sbt-ci-release branch December 27, 2022 08:07
@ckipp01 ckipp01 mentioned this pull request Dec 27, 2022
16 tasks
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.

4 participants