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

ci: Re-arrange workflows to remove duplication #410

Merged
merged 12 commits into from
Jan 3, 2025

Conversation

marcospereira
Copy link
Contributor

@marcospereira marcospereira commented Dec 5, 2024

Motivation

There are two primary motivations here:

  1. Allow to add jobs to the workflows easily. For example, it should be easy to test against a new JDK version or a new Kotlin version. We currently do that via matrices and steps. This brings all to matrices.
  2. Remove duplication: there are duplications regarding the matrix configuration and steps to set up caching.

The final workflow looks like this:

Screenshot 2024-12-04 at 11 23 50 PM

Reference: https://github.com/marcospereira/jte/actions/runs/12173115754

Main changes

Merging Graal and Maven workflows

They are mostly copied and pasted from each other, with minor differences, and bringing them together into a single workflow file helps to later maintain consistency in a single place.

Jobs dependencies

Previously, all the jobs were executed simultaneously, meaning that even if the code does not pass unit tests, it still runs coverage, graal tests, etc. Now, there is a dependency between jobs, so we only execute more expensive workflow sections if the tests pass.

This hasn't added much to the workflow total time.

Reusable actions

Two new reusable actions exist to set up Gradle caching and remove locally published artifacts before caching. Although it is possible to add a few other reusable actions for steps that seem repetitive, I've decided to add only these two because they don't add logic to the build. Instead, they are mainly CI infrastructure.

Future work

I will build on top of this to add support for Kotlin 2.1.0. It is currently broken for the Gradle plugin.

@marcospereira marcospereira changed the title ci/merge workflows ci: Re-arrange workflows to remove duplication Dec 5, 2024
@@ -18,4 +18,4 @@ jobs:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: gradle/wrapper-validation-action@v3
- uses: gradle/actions/wrapper-validation@v4
Copy link
Contributor Author

Choose a reason for hiding this comment

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

An small bonus to update this workflow to use the new wrapper-validation action.

@@ -94,7 +170,7 @@ jobs:
if: contains(matrix.os, 'win') == false
run: chmod +x ./mvnw
- name: Build with Maven
run: ./mvnw verify --file pom.xml -Pcoverage
run: ./mvnw verify --file pom.xml -Pcoverage,kotlin-1.9
Copy link
Contributor Author

Choose a reason for hiding this comment

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

kotlin-1.9 needs to be explicitly activated here, as documented here:

This profile will automatically be active for all builds unless another profile in the same POM is activated using one of the previously described methods. All profiles that are active by default are automatically deactivated when a profile in the POM is activated on the command line or through its activation config.

@@ -69,6 +68,22 @@
</plugins>
</build>
</profile>
<!-- Kotlin versions -->
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't like having this profile in two places, but I thought it was better than moving it to the parent pom.

Copy link

codecov bot commented Dec 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.33%. Comparing base (190ced3) to head (c934e26).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##               main     #410   +/-   ##
=========================================
  Coverage     91.33%   91.33%           
  Complexity     1215     1215           
=========================================
  Files            76       76           
  Lines          3161     3161           
  Branches        489      489           
=========================================
  Hits           2887     2887           
  Misses          164      164           
  Partials        110      110           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@nedtwigg
Copy link
Contributor

This looks great to me!

@casid
Copy link
Owner

casid commented Jan 3, 2025

Hi @marcospereira, this looks great! I really like, that we now have everything in one build file together.

And sorry for the late reply, I just returned from a long vacation.

@casid casid merged commit 6f5434e into casid:main Jan 3, 2025
15 checks passed
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