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

GitHub Actions CI template: do not skip intermediate builds on the main, master, or release-* branches #341

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

DilumAluthge
Copy link
Member

Follow-up to #325

@codecov
Copy link

codecov bot commented Feb 12, 2022

Codecov Report

Merging #341 (1974b1b) into master (1cb56bf) will decrease coverage by 15.44%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           master     #341       +/-   ##
===========================================
- Coverage   94.62%   79.17%   -15.45%     
===========================================
  Files          20       20               
  Lines         632      629        -3     
===========================================
- Hits          598      498      -100     
- Misses         34      131       +97     
Impacted Files Coverage Δ
src/plugins/develop.jl 0.00% <0.00%> (-100.00%) ⬇️
src/plugins/register.jl 0.00% <0.00%> (-100.00%) ⬇️
src/plugins/coverage.jl 0.00% <0.00%> (-90.91%) ⬇️
src/plugins/ci.jl 17.72% <0.00%> (-79.75%) ⬇️
src/plugins/citation.jl 20.00% <0.00%> (-60.00%) ⬇️
src/plugins/project_file.jl 82.35% <0.00%> (-11.77%) ⬇️
src/plugins/documenter.jl 75.36% <0.00%> (-11.60%) ⬇️
src/plugins/tagbot.jl 88.88% <0.00%> (-11.12%) ⬇️
src/plugins/tests.jl 90.00% <0.00%> (-10.00%) ⬇️
src/plugins/git.jl 90.16% <0.00%> (-3.39%) ⬇️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1cb56bf...1974b1b. Read the comment docs.

@DilumAluthge DilumAluthge force-pushed the dpa/do-not-skip-builds-on-the-main-branch branch from 47a582c to 1974b1b Compare February 12, 2022 12:00
@DilumAluthge
Copy link
Member Author

Bump @christopher-dG @nickrobinson251

Copy link
Collaborator

@nickrobinson251 nickrobinson251 left a comment

Choose a reason for hiding this comment

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

Seems sensible to me!

Looks like the reference tests need updating (they only run on Julia v1.7.1 which is why that CI job fails), then LGTM

Also please bump the version and I'll release straight away

Thanks!

group: ${{ github.workflow }}-${{ github.ref }}
# Skip intermediate builds: all builds except for builds on the `main`, `master`, or `release-*` branches
# Cancel intermediate builds: only pull request builds
group: ${{ github.workflow }}-${{ github.ref }}-${{ github.ref != 'refs/heads/main' || github.ref != 'refs/heads/master' || startsWith(github.ref, 'refs/heads/release-') || github.run_number }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

can you explain a little more what this does? I read https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#concurrency but I'm still a bit confused

Copy link
Member Author

Choose a reason for hiding this comment

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

So, roughly speaking, if two separate builds have the same concurrency group value, then the second build will have to wait until the first build has completed. Additionally, if three builds all have the same concurrency group value, then while the first build is happening, the second and third build will have to wait, but once the first build has completed, the second build will be completely skipped, and the third build will start.

This is good for PRs, but usually it is undesirable to not run CI on every commit pushed to main or master. Therefore, for commits on main, master, and release-*, we create a unique concurrency group value. This ensures that two different commits on main never have the same concurrency group value, and thus builds on main are never skipped.

For commits on PRs, two separate commits on the same PR will have the same concurrency group value. Therefore, if multiple commits are pushed to a PR, we can skip CI on the intermediate commits, and only run CI on the latest commit of the PR, since that is the code that we actually care about.

@DilumAluthge DilumAluthge force-pushed the dpa/do-not-skip-builds-on-the-main-branch branch from 1974b1b to c8edbe8 Compare June 21, 2024 18:34
@DilumAluthge DilumAluthge force-pushed the dpa/do-not-skip-builds-on-the-main-branch branch from c8edbe8 to a539016 Compare June 29, 2024 05:35
Copy link

codecov bot commented Jun 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.46%. Comparing base (f1c4302) to head (a539016).

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

HEAD has 40 uploads less than BASE
Flag BASE (f1c4302) HEAD (a539016)
52 12
Additional details and impacted files
@@             Coverage Diff             @@
##           master     #341       +/-   ##
===========================================
- Coverage   94.33%   77.46%   -16.88%     
===========================================
  Files          24       24               
  Lines         742      741        -1     
===========================================
- Hits          700      574      -126     
- Misses         42      167      +125     

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

@DilumAluthge
Copy link
Member Author

@gdalle @nickrobinson251 How would I go about fixing the CI failures here?

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