-
Notifications
You must be signed in to change notification settings - Fork 616
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: update workflow fork conditions to allow run on copies #6783
Conversation
This PR is a test. Don't merge! edit: It looks like this may be good to go! 🚀 |
It appears that updating the The changes in this repo allows the e2e-fork workflow to run on both forks and copies of the repository. Specifically, if we're running on |
cancel-in-progress: ${{ github.ref != 'refs/heads/main' }} | ||
|
||
jobs: | ||
# dynamically build a matrix of test/test suite pairs to run | ||
build-test-matrix: | ||
if: ${{ github.event.pull_request.head.repo.fork || github.actor == 'dependabot[bot]' || github.event_name == 'workflow_dispatch' }} | ||
if: ${{ github.repository != 'cosmos/ibc-go' || github.actor == 'dependabot[bot]' || github.event_name == 'workflow_dispatch' }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fine with either but is this required? I'd assume checking if it is/isn't a fork was the most general option?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to use something like a "private fork", it is not treated as an actual fork
quirky af. Makes sense to me, want to add a small comment there explaining it? Can see this as being something someone would change in future w/o knowing why it was there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm considering it runs just fine on your fork. thanks for looking into github shenanigans!
@@ -11,13 +11,13 @@ on: | |||
|
|||
# cancel workflows if a new one is triggered on the same branch. | |||
concurrency: | |||
group: ${{ github.ref }} | |||
group: ${{ github.workflow }}-${{ github.ref }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we want to amend similar line in e2e-wasm
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Solution is a bit hacky but I think is actually not too bad working within the restraints we have for the triggers.
Description
Updates E2E fork conditions to allow
e2e-fork
to also run on copies.See: https://github.com/damiannolan/ibc-go-test/pull/9
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/
).godoc
comments.Files changed
in the GitHub PR explorer.SonarCloud Report
in the comment section below once CI passes.