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

Add concurrency to the GitHub workflows to auto-cancel older runs of actions in PRs. #5328

Merged
merged 19 commits into from
Feb 28, 2022

Conversation

langleyd
Copy link
Member

Resolves #5327

@langleyd langleyd self-assigned this Feb 23, 2022
@langleyd langleyd marked this pull request as draft February 23, 2022 18:02
@github-actions
Copy link

github-actions bot commented Feb 23, 2022

Matrix SDK

Integration Tests Results:

  • [org.matrix.android.sdk.session]
    passed="25" failures="17" errors="0" skipped="2"
  • [org.matrix.android.sdk.account]
    passed="5" failures="0" errors="0" skipped="2"
  • [org.matrix.android.sdk.internal]
    passed="158" failures="12" errors="0" skipped="38"
  • [org.matrix.android.sdk.ordering]
    passed="16" failures="0" errors="0" skipped="0"
  • [org.matrix.android.sdk.PermalinkParserTest]
    passed="2" failures="0" errors="0" skipped="0"

@github-actions
Copy link

github-actions bot commented Feb 23, 2022

Unit Test Results

  84 files  ±0    84 suites  ±0   1m 4s ⏱️ +12s
157 tests ±0  157 ✔️ ±0  0 💤 ±0  0 ±0 
504 runs  ±0  504 ✔️ ±0  0 💤 ±0  0 ±0 

Results for commit c61af45. ± Comparison against base commit 8d5d064.

♻️ This comment has been updated with latest results.

@langleyd langleyd changed the title Add concurrency to integration tests. Add concurrency to the GitHub workflows to auto-cancel older runs of actions in PRs. Feb 23, 2022
@langleyd langleyd marked this pull request as ready for review February 23, 2022 23:20
Copy link
Contributor

@michaelkaye michaelkaye left a comment

Choose a reason for hiding this comment

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

Intent of PR is great, looks good.

A few nits around naming (to keep consistency), but I think we have a correctness issue with the missing matrix.target

Also wondering if we should add comments for the other workflows as well (Even if it in "sanity_test.yml" it just says run every time") so we are reminded to consider it in future.

.github/workflows/quality.yml Outdated Show resolved Hide resolved
.github/workflows/quality.yml Outdated Show resolved Hide resolved
.github/workflows/tests.yml Outdated Show resolved Hide resolved
langleyd and others added 4 commits February 24, 2022 16:26
Co-authored-by: Michael Kaye <1917473+michaelkaye@users.noreply.github.com>
Co-authored-by: Michael Kaye <1917473+michaelkaye@users.noreply.github.com>
Co-authored-by: Michael Kaye <1917473+michaelkaye@users.noreply.github.com>
@langleyd
Copy link
Member Author

Nice suggestions @michaelkaye , thanks for the review!

Copy link
Contributor

@michaelkaye michaelkaye left a comment

Choose a reason for hiding this comment

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

This looks good to me.

@ouchadam
Copy link
Contributor

out of interest, what's the reasoning for needing more than the head ref? group: ${{ github.head_ref }}

@michaelkaye
Copy link
Contributor

out of interest, what's the reasoning for needing more than the head ref? group: ${{ github.head_ref }}

Groups cross workflows, so the per-workflow name is required to avoid concurrency blocking across workflows ; the instances where we use the SHA hash is to ensure multiple runs on the same workflow can happen; the instances where the matrix target is also included is when we want both sides of the target to run.

(never used this before, but that was my understanding when i reviewed)

@ouchadam
Copy link
Contributor

thanks for the explanation!

@bmarty bmarty merged commit d5cb315 into develop Feb 28, 2022
@bmarty bmarty deleted the feature/dla/ci_check_concurrency branch February 28, 2022 13:20
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.

Add concurrency to the GitHub workflows to auto-cancel older runs of some actions in PRs.
4 participants