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

Support multiple Play json versions concurrently #37

Merged
merged 1 commit into from
Dec 21, 2023

Conversation

ioannakok
Copy link
Contributor

@ioannakok ioannakok commented Dec 15, 2023

What does this change?

  • Supports multiple play-json versions concurrently
  • Also, adopts GHA Scala library release workflow
  • Changes the artifact coordinates from "com.gu" %% "targeting-client" % to "com.gu.targeting-client" %% "client-play-json-vXX"

Why?

These are the consumers of this library:

  • frontend: Scala 2.13, Play 2.8 & 3.0
  • MAPI: Scala 2.13, Play 2.8
  • targeting: Scala 2.13, Play 2.8

We now want to upgrade frontend to Play 3: guardian/frontend#26755 but we also need to ensure that versions of this library are available to consumers that are stil on Play 2.8.

The updated sbt configuration is based on techniques used in https://github.com/guardian/facia-scala-client and https://github.com/guardian/play-googleauth where it's been successful for a few years now.

These are the consumers of this library:
* frontend: Scala 2.13, Play 2.8 & 3.0
* MAPI: Scala 2.13, Play 2.8
* targeting: Scala 2.13, Play 2.8

We now want to upgrade frontend to Play 3: guardian/frontend#26755 but we also need to ensures that versions of this library are available to consumers that are stil on Play 2.8.

The updated sbt configuration is based on techniques used in https://github.com/guardian/facia-scala-client and https://github.com/guardian/play-googleauth where it's been successful for a few years now.

Co-authored-by: Roberto Tyley <roberto.tyley@guardian.co.uk>
@ioannakok ioannakok force-pushed the support-multiple-play-json-versions-concurrently branch from b007690 to 1233618 Compare December 15, 2023 17:48
@ioannakok ioannakok marked this pull request as ready for review December 15, 2023 17:56
@alinaboghiu alinaboghiu added this to the Health milestone Dec 19, 2023
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.

I reckon we can probably proceed with this PR if you're happy with it Ioanna? No worries if you've got someone lined up to do a review tho! I guess @sophie-macmillan & @abeddow91 were the most recent contributors to this library, maybe one of them could take a quick look?

@ioannakok ioannakok requested a review from a team December 20, 2023 11:26
@ioannakok ioannakok merged commit 862c112 into main Dec 21, 2023
ioannakok added a commit that referenced this pull request Dec 21, 2023
After #37 `targeting-client` supports multiple Play versions concurrently. PR #37 also changed the group id and added `release.yml`. The new workflow performs releases using this library: https://github.com/guardian/gha-scala-library-release-workflow. This PR updates the `README` to reflect these changes.
@ioannakok ioannakok mentioned this pull request Dec 21, 2023
@@ -0,0 +1,13 @@
name: Release
Copy link
Member

Choose a reason for hiding this comment

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

Oh, we made a mistake here! The file should be at:

.github/workflows/release.yml

...not...

.github/release.yml

...GitHub doesn't recognise the workflow right now because this file is in the wrong place - so 'Release' isn't listed here:

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, here's a PR to fix this: #39

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 first release failed because we changed the Maven coordinates of the artifact so now sbt-version-policy plugin cannot find the previous version. We've raised #40 to fix this.

ioannakok added a commit that referenced this pull request Dec 22, 2023
The first release failed https://github.com/guardian/targeting-client/actions/runs/7299624335/job/19892813494 because in #37 we changed the coordinates of the artifact so now `sbt-version-policy` plugin cannot find the previous version. Once we perform a release we will put back this line. See also guardian/facia-scala-client#300 (comment)

Co-authored-by: Roberto Tyley <roberto.tyley@guardian.co.uk>
rtyley added a commit to guardian/gha-scala-library-release-workflow that referenced this pull request Dec 22, 2023
We had a failed release on https://github.com/guardian/targeting-client after
guardian/targeting-client#37 accidentally removed the
`license` setting:

https://github.com/guardian/targeting-client/actions/runs/7299896704/job/19893644176#step:5:56
```
2023-12-22 12:37:31.245Z error [SonatypeClient] [close] Failed  - (SonatypeClient.scala:181)
2023-12-22 12:37:31.245Z error [SonatypeClient] Activity name:close, started:2023-12-22T12:36:34.296Z  - (SonatypeClient.scala:469)
2023-12-22 12:37:31.246Z error [SonatypeClient]     Failed: pom-staging, failureMessage:Invalid POM: /com/gu/targeting-client/client-play-json-v30_2.13/1.1.5/client-play-json-v30_2.13-1.1.5.pom: License information missing  - (SonatypeClient.scala:387)
```

Specifying a license is *required* for submitting a project to Maven Central:

https://central.sonatype.org/publish/requirements/#license-information

Note that sbt v1.6.2 and above (thanks to sbt/librarymanagement#395)
supply a `License` object that means you can define the license much more concisely:

```
licenses := Seq("Apache-2.0" -> url("https://www.apache.org/licenses/LICENSE-2.0"))
```

Used successfully in guardian/etag-caching#29
rtyley added a commit to guardian/gha-scala-library-release-workflow that referenced this pull request Dec 22, 2023
We had a failed release on https://github.com/guardian/targeting-client after
guardian/targeting-client#37 accidentally removed the
`license` setting:

https://github.com/guardian/targeting-client/actions/runs/7299896704/job/19893644176#step:5:56
```
2023-12-22 12:37:31.245Z error [SonatypeClient] [close] Failed  - (SonatypeClient.scala:181)
2023-12-22 12:37:31.245Z error [SonatypeClient] Activity name:close, started:2023-12-22T12:36:34.296Z  - (SonatypeClient.scala:469)
2023-12-22 12:37:31.246Z error [SonatypeClient]     Failed: pom-staging, failureMessage:Invalid POM: /com/gu/targeting-client/client-play-json-v30_2.13/1.1.5/client-play-json-v30_2.13-1.1.5.pom: License information missing  - (SonatypeClient.scala:387)
```

Specifying a license is *required* for submitting a project to Maven Central:

https://central.sonatype.org/publish/requirements/#license-information

Note that sbt v1.6.2 and above (thanks to sbt/librarymanagement#395)
supply a `License` object that means you can define the license much more concisely:

```
licenses := Seq(License.Apache2)
```

Used successfully in guardian/etag-caching#29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants