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

Update build and include sbt publish GHA workflow #193

Merged
merged 5 commits into from
Apr 16, 2024

Conversation

adamnfish
Copy link
Contributor

What does this change?

Sets up Anghammarad to use the sbt publish GHA workflow, removing redundant publishing plugins for sbt and eliminating sbt config options that are no longer required.

We also include the workflow file itself, taken from the docs.

https://github.com/guardian/gha-scala-library-release-workflow/blob/main/docs/configuration.md

How to test

We'll hit the "release" button once this (and dependent infrastructure*) is merged.

  • Note: the PR that provides access to credentials has not yet been merged, so this remains in draft.

@adamnfish adamnfish requested review from rtyley and tjsilver April 15, 2024 16:30
Copy link

changeset-bot bot commented Apr 15, 2024

⚠️ No Changeset found

Latest commit: 57e67b4

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@adamnfish adamnfish marked this pull request as ready for review April 16, 2024 10:16
Copy link
Contributor

@jacobwinch jacobwinch left a comment

Choose a reason for hiding this comment

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

🎉

build.sbt Outdated
@@ -19,21 +22,12 @@ inThisBuild(Seq(
scalacOptions ++= Seq(
"-deprecation",
"-Xfatal-warnings",
"-encoding", "UTF-8"
"-encoding", "UTF-8",
"-release:11",
),
// sonatype metadata
organization := "com.gu",
licenses := Seq("Apache V2" -> url("https://www.apache.org/licenses/LICENSE-2.0.html")),
Copy link
Member

Choose a reason for hiding this comment

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

This can be tweaked to:

Suggested change
licenses := Seq("Apache V2" -> url("https://www.apache.org/licenses/LICENSE-2.0.html")),
licenses := Seq(License.Apache2),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah thanks, might as well update that now as well 👍

@@ -0,0 +1,13 @@
name: Release Scala client
Copy link
Member

Choose a reason for hiding this comment

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

Just by convention, this file is called release.yml in all the other repos that have gha-scala-library-release-workflow - would it be ok to rename this file to match?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The trouble is that this repo has a node client and a Scala client. I didn't want the workflow to give the impression it would do both. Let me know if you feel super strongly, but I don't think hogging the word "release" in the workflow namespace is always a good idea!

Copy link
Member

Choose a reason for hiding this comment

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

That makes sense - what we've actually done with other libraries that publish both NPM & Scala packages is that we've tied them together in a single release workflow (at which point, it's kind of ok to just call it release.yml again). We allow the NPM release to utilise the Scala libraries version number, so that their releases are synchronised and it's easy to see whether the NPM and Scala libraries are from the same point in time.

We've done this for:

image

These libraries where largely completely defined by their thrift definitions, so if one library wanted to change, the other did too. This might not be the case with anghammarad, so it might not apply here, but it is quite nice to just have one button to press.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok makes sense! I'll rename it and we can consider unifying these libraries in the future.

Sets up Anghammarad to use the sbt publish GHA workflow, removing
redundant publishing plugins for sbt and eliminating sbt config
options that are no longer required.

We also include the workflow file itself, taken from the docs.

https://github.com/guardian/gha-scala-library-release-workflow/blob/main/docs/configuration.md
Copy link
Contributor

@tjsilver tjsilver left a comment

Choose a reason for hiding this comment

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

We should update the README about releasing the Scala client as well. I also agree that weaving the NPM release into the release workflow as suggested by Roberto would be a good idea, if appropriate. Both can be done as separate PRs though.

This repository contains an npm client library as well as the scala
client. Despite this, we're best off creating a single "release"
workflow and if we want to automate the npm flow we can have it be
called from this single release workflow.
Copy link
Contributor

@tjsilver tjsilver left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Member

@rtyley rtyley left a comment

Choose a reason for hiding this comment

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

Looks good! One small tweak to the readme that you can have if you like.

README.md Outdated Show resolved Hide resolved
Improve the documentation of the release workflow

Co-authored-by: Roberto Tyley <[email protected]>
@adamnfish adamnfish merged commit 365f0aa into main Apr 16, 2024
1 check passed
@adamnfish adamnfish deleted the migrate-to-sbt-release-workflow branch April 16, 2024 15:34
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