Skip to content

JDBetteridge/Composite actions#2868

Open
JDBetteridge wants to merge 11 commits intomainfrom
JDBetteridge/unique_ci_docker_tag
Open

JDBetteridge/Composite actions#2868
JDBetteridge wants to merge 11 commits intomainfrom
JDBetteridge/unique_ci_docker_tag

Conversation

@JDBetteridge
Copy link
Contributor

@JDBetteridge JDBetteridge commented Mar 6, 2026

Create composite actions for executing Docker things in workflows.

Makes stuff more consistent

The following workflows were renamed to be more descriptive/consistent:

  • pytest-mpi (gcc) → pytest-mpi (pytest-mpi-docker-gcc)
  • pytest-mpi (icx) → pytest-mpi (pytest-mpi-docker-icx)

Which needs updating in the repo settings

@codecov
Copy link

codecov bot commented Mar 7, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.48%. Comparing base (e147aaa) to head (0a51941).

❗ There is a different number of reports uploaded between BASE (e147aaa) and HEAD (0a51941). Click for more details.

HEAD has 17 uploads less than BASE
Flag BASE (e147aaa) HEAD (0a51941)
18 1
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #2868       +/-   ##
===========================================
- Coverage   79.05%   61.48%   -17.58%     
===========================================
  Files         248      193       -55     
  Lines       51276    31363    -19913     
  Branches     4431     3805      -626     
===========================================
- Hits        40534    19282    -21252     
- Misses       9932    11136     +1204     
- Partials      810      945      +135     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@JDBetteridge JDBetteridge force-pushed the JDBetteridge/unique_ci_docker_tag branch 4 times, most recently from 1ac1672 to 74a1f49 Compare March 16, 2026 14:57
Comment on lines +7 to +10
uid:
description: "Unique identifier output from docker-build action"
required: true
tag:
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 pass both uid and tag everywhere, should these be combined into a single output from build and single input for clean and run? A unique_tag?

required: true
command:
description: "Command to execute inside of the docker container"
required: true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A limitation of this approach is that only one command can be run in a container at once, making the workflows a little verbose in some places.

@JDBetteridge JDBetteridge changed the title JDBetteridge/unique ci docker tag JDBetteridge/Composite actions Mar 16, 2026
@JDBetteridge JDBetteridge marked this pull request as ready for review March 16, 2026 15:07
@JDBetteridge JDBetteridge force-pushed the JDBetteridge/unique_ci_docker_tag branch 4 times, most recently from fd7bcda to 2b457f6 Compare March 16, 2026 16:20
OMP_NUM_THREADS: 2

strategy:
# Prevent all build to stop if a single one fails
Copy link
Contributor

Choose a reason for hiding this comment

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

Might as well correct grammar on this comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All your base are belong to us

Copy link
Contributor

Choose a reason for hiding this comment

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

Showing your advanced age there...

@JDBetteridge JDBetteridge force-pushed the JDBetteridge/unique_ci_docker_tag branch from ae9f243 to 5835ad4 Compare March 16, 2026 19:59
Copy link
Contributor

@mloubout mloubout left a comment

Choose a reason for hiding this comment

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

Looks great, thanks a lot!!

Left a bunch of generic comments that aren't strong change requests. Do those action files really need to be in three different directories? Why not just .github/actions/docker for all

strategy:
# Prevent cancellation if a single workflow fails
fail-fast: false
matrix:
Copy link
Contributor

Choose a reason for hiding this comment

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

It's a single entry, can drop the matrix and just put the values where needed, e.g

  tutorials-docker:
    name: tutos-docker-gcc-py310
    runs-on: $ubuntu-latest

and so on

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 agree, but I was also going to raise the possibility of testing with some newer compilers in the next dev meeting, so I left it like this to be extensible (and consistent)

Comment on lines +91 to +97
- id: build
name: Build docker image
uses: ./.github/actions/docker-build
with:
file: docker/Dockerfile.devito
tag: ${{ matrix.name }}
base: devitocodes/bases:cpu-${{ matrix.arch }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mloubout here we don't use base

@JDBetteridge JDBetteridge force-pushed the JDBetteridge/unique_ci_docker_tag branch from 9d3406a to 1f75496 Compare March 20, 2026 15:19
@JDBetteridge
Copy link
Contributor Author

Do those action files really need to be in three different directories

Sadly yes, you cannot use them otherwise 😞

@JDBetteridge JDBetteridge force-pushed the JDBetteridge/unique_ci_docker_tag branch from 1f75496 to 0a51941 Compare March 20, 2026 15:24
@mloubout mloubout added testing CI continuous integration installation labels Mar 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI continuous integration installation testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants