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

Introduce CI workflow logic #195

Merged
merged 13 commits into from
Nov 26, 2024
Merged

Introduce CI workflow logic #195

merged 13 commits into from
Nov 26, 2024

Conversation

jwallwork23
Copy link
Contributor

Closes #194.

In the interests of green computing and avoiding hitting the limit of the free-plan, this PR introduces some logic to only run workflows if relevant changes are made.

  • Only run the tests if source files, CMakeLists.txt, requirements.txt, or data are changed.
  • Only run the static analysis workflow if C, C++, Fortran, or Python files have changed.
  • Only run the Fypp checks if Fortran files have changed.
  • Only build the docs if Markdown or content in pages/ have changed.

If any of the workflow files are changed then those tests should also be run.

The PR also improves the consistency of formatting across the workflow configuration files.

@jwallwork23 jwallwork23 added the testing Related to FTorch testing label Nov 25, 2024
@jwallwork23 jwallwork23 self-assigned this Nov 25, 2024
@jwallwork23
Copy link
Contributor Author

Some checks:

  • My initial push 00bee3e didn't run any tests because no source code or docs have been changed.
  • My second push de42e78 added a check for if fypp_checks.yml changed. This file has changed in the PR so those tests were run.
  • My third push d154d14 added a check for if static_analysis.yml changed, which it has. So both Fypp and linting checks were run.
  • (You get the picture) 3ba39ca added build_docs.yml check, so Fypp, static analysis and build docs were run.
  • Finally c1f6a04 added test_suite.yml check, so all four were run.

@jwallwork23 jwallwork23 marked this pull request as ready for review November 25, 2024 10:34
Copy link
Member

@jatkinson1000 jatkinson1000 left a comment

Choose a reason for hiding this comment

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

This all seems sensible @jwallwork23
A couple of stylistic comments and a suggestion to remove the options you had already flagged.

.github/workflows/build_docs.yml Outdated Show resolved Hide resolved
.github/workflows/fypp_checks.yml Outdated Show resolved Hide resolved
Copy link
Member

@jatkinson1000 jatkinson1000 left a comment

Choose a reason for hiding this comment

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

LGTM, have added suggestions for formatting of the other branch coverage lists.

.github/workflows/fypp_checks.yml Outdated Show resolved Hide resolved
.github/workflows/static_analysis.yml Outdated Show resolved Hide resolved
.github/workflows/test_suite.yml Outdated Show resolved Hide resolved
@jwallwork23 jwallwork23 merged commit 20335ed into main Nov 26, 2024
5 checks passed
@jwallwork23 jwallwork23 deleted the 194_ci-test-logic branch November 26, 2024 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing Related to FTorch testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Only run tests if code has changed
2 participants