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.

Sign up for GitHub

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

Run CI only when useful #1372

Closed
wants to merge 1 commit into from
Closed

Run CI only when useful #1372

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .github/workflows/ci.yaml
View file
Open in desktop
Copy link
Member Author

Choose a reason for hiding this comment

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

Re #1372 (comment):

Unrelated to those changes here: Would be great to have a per workflow opt out option in case we know a certain workflow doesn't work with certain CI steps, like rsv with conda.

Isn't that what we're doing with 724e207, or do you mean something else?

Copy link
Member

Choose a reason for hiding this comment

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

It disables both docker and conda if I'm not mistaken. But not a big deal, we have lots of redundancy so disabling both is fine.

Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ on:

workflow_dispatch:

# Prevent concurrent runs on the same branch, except for the default branch.
concurrency:
group: ${{ github.workflow }}-${{ github.ref }}-${{ github.ref_type == 'branch' && github.ref_name == github.event.repository.default_branch && github.run_id }}
cancel-in-progress: true
Comment on lines +16 to +19
Copy link
Member Author

Choose a reason for hiding this comment

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

I have a counter-argument to ea5104f, describing a situation where concurrency is beneficial:

Sometimes, it's useful to show a difference in CI results between two commits. I have found myself pushing one commit immediately followed by another and relying on CI to run without interruption on both. With the proposed change, the same effect can only be accomplished in two ways:

  1. Wait for the first commit's CI run to finish before pushing the next.
  2. Temporarily remove this concurrency group in the PR.

I would rather cancel unwanted concurrent runs manually than use either of those approaches. Since ea5104f is the only commit left in this PR, I'm just going to close it.


jobs:
pytest-cram:
name: test (python=${{ matrix.python-version }} biopython=${{ matrix.biopython-version || 'latest' }})
Expand Down
Loading