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

Install Java and sbt in dependency-graph workflow #522

Merged
merged 1 commit into from
Dec 12, 2024

Conversation

emdash-ie
Copy link
Contributor

What does this change?

The sbt-dependency-graph workflow has started failing, because it uses ubuntu-latest but doesn’t install Java or sbt: since github have updated ubuntu-latest to point to ubuntu-24.04, it has less software pre-installed by default.

The updates come from copying the version in
https://github.com/guardian/content-api-transformer-lambda/pull/95, which was raised by the bot recently and has been updated to install the necessary dependencies.

How to test

We could alter the triggers for the workflow to let it run on this branch in order to test it, but since it's working correctly on content-api-transformer-lambda already, I'm tempted to just merge and see.

The sbt-dependency-graph workflow has started failing, because it uses
ubuntu-latest but doesn’t install Java or sbt: since github have updated
ubuntu-latest to point to ubuntu-24.04, it has less software
pre-installed by default.

The updates come from copying the version in
guardian/content-api-transformer-lambda#95,
which was raised by the bot recently and has been updated to install the
necessary dependencies.
@emdash-ie emdash-ie requested a review from a team as a code owner December 12, 2024 10:10
Copy link
Member

@johnduffell johnduffell left a comment

Choose a reason for hiding this comment

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

👍

@johnduffell johnduffell merged commit ea5c70d into main Dec 12, 2024
1 check passed
@johnduffell johnduffell deleted the fix-sbt-dependency-graph branch December 12, 2024 10:14
Comment on lines -13 to +22
uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
uses: actions/checkout@eef61447b9ff4aafe5dcd4e0bbf5d482be7e7871 # v4.2.1
- name: Install Java
id: java
uses: actions/setup-java@b36c23c0d998641eff861008f374ee103c25ac73 # v4.2.0
with:
distribution: corretto
java-version: 17
- name: Install sbt
id: sbt
uses: sbt/setup-sbt@8a071aa780c993c7a204c785d04d3e8eb64ef272 # v1.1.0
Copy link
Member

@rtyley rtyley Dec 12, 2024

Choose a reason for hiding this comment

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

Hi - could we use guardian/setup-scala instead? This project already has .tool-versions file, and from @adamnfish's email on 4th December, DevX do plan to use guardian/setup-scala:

To summarise, we are replacing the standard actions/setup-java step with our own Guardian-managed guardian/setup-scala step. This new step installs Java and Scala (sbt) and simplifies the configuration of our runners by aligning with other parts of our tooling. In particular, the Java version is no longer provided directly in the workflow file. Instead, this comes from a .tool-versions file at the root of the repository.

I'm happy to make the changes (also following on from guardian/play-googleauth#275 & guardian/simple-configuration#103), just don't want to clash if there's a reason to go another way!?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Go for it! I only didn't use it because the example I was copying from (raised by the dependency integrator bot) didn't use setup-scala – I don't have any strong feelings about it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(Looking at dates, the integrator bot raised the PR I copied from on the 2nd of December, so presumably it just predates the decision to use setup-scala)

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.

3 participants