fix(gh-workflow): skip comment when push directly#6179
fix(gh-workflow): skip comment when push directly#6179awxiaoxian2020 wants to merge 1 commit intoreactjs:mainfrom
Conversation
Size changesDetails📦 Next.js Bundle Analysis for react-devThis analysis was generated by the Next.js Bundle Analysis action. 🤖 This PR introduced no changes to the JavaScript bundle! 🙌 |
|
Who can follow this PR? |
|
PTAL. |
| if: > | ||
| ${{ github.event.workflow_run.event == 'pull_request' && | ||
| github.event.workflow_run.conclusion == 'success' }} |
There was a problem hiding this comment.
I believe the core problem lies in the evaluation of these lines by GitHub's runner. If github.event.workflow_run.event is not even the right parameter (as you implied in another PR), then this job should never run. Nevertheless, it runs, meaning that the condition is always set to true somehow - just like here: actions/runner#1173. Presumably, ${{ and }} syntax along with block formatting sign > result in the text, thus making it a truthy value.
Did you try other variations of that condition without the block sign > and with event_name like this?
if: github.event.workflow_run.event_name == 'pull_request' && github.event.workflow_run.conclusion == 'success'P.S.
Regarding the PR itself, I think we still need the if success check for the entire comment job. Otherwise, the remaining Comment step is going to produce an error in case steps.get-comment-body.outputs is empty.
P.P.S.
Also, I was thinking that ignoring the default branch for this workflow can serve as an additional precaution as well:
on:
workflow_run:
workflows: ["Analyze Bundle"]
types:
- completed
branches-ignore:
- main # change this if your default branch is named differentlyThere was a problem hiding this comment.
I actually checked and this did the trick:
if: github.event.workflow_run.event == 'pull_request' && github.event.workflow_run.conclusion == 'success'event is the right property and the evaluation is indeed broken.
There was a problem hiding this comment.
I actually checked and this did the trick:
How do you check this?
There was a problem hiding this comment.
@alinkedd I remember > was used for multiline?
also I checked what other option we have for multiline and ended up here https://github.com/orgs/community/discussions/25641#discussioncomment-3248571 if we try with | and everything works properly then the problem might be with > only?
There was a problem hiding this comment.
@awxiaoxian2020 I tried hypothesis with same condition in my test repo and it seemed to be right.
UPD
The fixes for workflows triggered by workflow_run work only if they are added to the default branch, so I could not tested them here with PR.
There was a problem hiding this comment.
@harish-sethuraman
It seems, the problem is not about block signs (|, >, etc), but rather about expression within ${{ and }} and some text around it (even if it's | or >). So today I rechecked this more readable version without expression and it works fine:
if: >
github.event.workflow_run.event == 'pull_request' &&
github.event.workflow_run.conclusion == 'success'And I updated my another PR #6651 which also contains some other fixes
|
@alinkedd I'd love to help here but i really just don't understand these workflows enough to review. @gaearon or @harish-sethuraman do you know? |
|
@rickhanlonii |
|
closing as resolved |
5 participants
close #5801.
The workaround is fine in zh-hans.react.dev now.