-
Notifications
You must be signed in to change notification settings - Fork 111
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
RSDK-577 - allow appimage in PR to ignore test results #1708
Conversation
@@ -5,7 +5,6 @@ concurrency: | |||
cancel-in-progress: true | |||
|
|||
on: | |||
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.
we aren't using this
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.
[opt] I believe I put this in there originally because "why not?" Made for easier testing if/when needed. Fine with removal though.
This was AWFUL to test |
|
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.
Conditionals are... ugh. Github workflows are a pain always. But I trust you got this sorted out, and nothing leaps out at me. Do note it's nearly 1am on christmas eve, so you may not be getting my most critical eye. :-)
@@ -5,7 +5,6 @@ concurrency: | |||
cancel-in-progress: true | |||
|
|||
on: | |||
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.
[opt] I believe I put this in there originally because "why not?" Made for easier testing if/when needed. Fine with removal though.
This adds the
appimage-ignore-tests
label that will still build an image despite test results (basically always) and can be used in conjunction withappimage
although it's pointless.This additionally cleans up the logic in our PR workflow such that:
test
when we receive thesafe to test
label (this happens per commit) or theappimage
label (needed for when the label is added after tests run since it's difficult to have a workflow look at a past action run's results; without it, the test job is always skipped)slam_integration_test
when we receive thesafe to test label
appimage
runs conditionally based on test success as expectedappimage-ignore-tests
runs conditionally on the same label and does not wait for tests