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: avoid duplicate runs for secondary branches on main repo #5308

Merged
merged 2 commits into from
Oct 2, 2024

Conversation

EZoni
Copy link
Member

@EZoni EZoni commented Sep 23, 2024

When pre-commit gets auto-updated, we typically see a PR from the branch pre-commit-ci-update-config.

This branch is created directly in the main fork (unlike the individual branches that WarpX contributors create from their own forks) and all CI checks run twice:

  • once for the activity on the PR that pre-commit automatically opens (these CI checks are labeled "PR automated")
  • once for the activity on the branch pre-commit-ci-update-config (these CI checks are labeled "individual CI")

Here's an example:
Screenshot from 2024-09-23 15-54-57

On top of this, once the PR is merged, CI runs a third time, because the merge is counted as activity on the branch development (again "individual CI").

We should be able to safely skip "individual CI" for the activity on the branch pre-commit-ci-update-config. This PR should do the trick, although it's good to double check the syntax for GitHub Actions and Azure pipelines.

My understanding is that the cleanup-cache and post-PR workflows don't need to be updated, also to be double checked.

@EZoni EZoni added component: tests Tests and CI cleaning Clean code, improve readability labels Sep 23, 2024
@EZoni EZoni requested a review from ax3l September 23, 2024 23:59
@ax3l ax3l self-assigned this Sep 30, 2024
Copy link
Member

@ax3l ax3l left a comment

Choose a reason for hiding this comment

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

Thanks! The nightly branch actually has its own CI scripts (and tries to install WarpX using various methods we document on various platforms)

.azure-pipelines.yml Outdated Show resolved Hide resolved
.github/workflows/clang_sanitizers.yml Outdated Show resolved Hide resolved
.github/workflows/clang_tidy.yml Outdated Show resolved Hide resolved
.github/workflows/cuda.yml Outdated Show resolved Hide resolved
.github/workflows/hip.yml Outdated Show resolved Hide resolved
.github/workflows/intel.yml Outdated Show resolved Hide resolved
.github/workflows/macos.yml Outdated Show resolved Hide resolved
.github/workflows/source.yml Outdated Show resolved Hide resolved
.github/workflows/ubuntu.yml Outdated Show resolved Hide resolved
.github/workflows/windows.yml Outdated Show resolved Hide resolved
This branch is a git "orphan" branch
that has its own scripts in it.
Copy link
Member

@ax3l ax3l left a comment

Choose a reason for hiding this comment

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

Thank you! ✨

@ax3l ax3l enabled auto-merge (squash) October 1, 2024 22:31
@ax3l ax3l merged commit 025f70e into ECP-WarpX:development Oct 2, 2024
37 checks passed
@EZoni EZoni deleted the ci_run_filter_branches branch October 2, 2024 06:42
@EZoni
Copy link
Member Author

EZoni commented Oct 2, 2024

Thanks! The nightly branch actually has its own CI scripts (and tries to install WarpX using various methods we document on various platforms)

Ah, I see. Does it have to be handled in a branch separate from development or is it just a legacy implementation? I'm asking because I realize that I never look at the nightly branch, so I don't perform any maintenance on that branch on my end. I would expect that we should be able to set up and maintain nightly workflows in the development branch as well. Would there be arguments against doing this?

@EZoni EZoni changed the title CI: avoid duplicate runs for secondary branches on main fork CI: avoid duplicate runs for secondary branches on main repo Oct 14, 2024
EZoni added a commit that referenced this pull request Oct 15, 2024
The fix introduced in #5308 was not correct for Azure pipelines.

In GitHub Actions we trigger a run on the `push` event only for the
`development` branch.

The Azure equivalent of that is triggering a run on the `trigger` event
only for the `development` branch. However, since the `trigger` event
was completely absent from the Azure pipeline file (that is, the default
setup was being used), I had erroneously added the filter branch to the
`pr` event instead, unlike what I did for GitHub actions where the
`push` was exposed in the YAML files.

This was originally aimed at avoiding duplicate runs for "individual CI"
when `pre-commit` opens a pull request by pushing to a secondary branch
`pre-commit-ci-update-config` in the main repo (instead of a fork).

The new setup is tested in #5393, where I copied these changes and where
one can see that a commit pushed to that PR does not trigger an
"individual CI" Azure pipeline anymore, but only a "PR automated" one.

Hopefully this is correct for the merge commits that get pushed to
`development` once a PR is closed, but we'll be able to test this only
after merging a PR.
dpgrote pushed a commit to dpgrote/WarpX that referenced this pull request Oct 23, 2024
…pX#5394)

The fix introduced in ECP-WarpX#5308 was not correct for Azure pipelines.

In GitHub Actions we trigger a run on the `push` event only for the
`development` branch.

The Azure equivalent of that is triggering a run on the `trigger` event
only for the `development` branch. However, since the `trigger` event
was completely absent from the Azure pipeline file (that is, the default
setup was being used), I had erroneously added the filter branch to the
`pr` event instead, unlike what I did for GitHub actions where the
`push` was exposed in the YAML files.

This was originally aimed at avoiding duplicate runs for "individual CI"
when `pre-commit` opens a pull request by pushing to a secondary branch
`pre-commit-ci-update-config` in the main repo (instead of a fork).

The new setup is tested in ECP-WarpX#5393, where I copied these changes and where
one can see that a commit pushed to that PR does not trigger an
"individual CI" Azure pipeline anymore, but only a "PR automated" one.

Hopefully this is correct for the merge commits that get pushed to
`development` once a PR is closed, but we'll be able to test this only
after merging a PR.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleaning Clean code, improve readability component: tests Tests and CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants