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

Jump to bottom

[GH workflow] update analyze.yml to avoid noisy notification #5801

Open
awxiaoxian2020 opened this issue Mar 23, 2023 · 5 comments · May be fixed by #6179 or #7667
Open

[GH workflow] update analyze.yml to avoid noisy notification #5801

awxiaoxian2020 opened this issue Mar 23, 2023 · 5 comments · May be fixed by #6179 or #7667

Comments

@awxiaoxian2020
Copy link
Contributor

When I merge PR into main branch, I always received the notification that the workflow failed like this.

I found the PR workflow is different from main branch. The latter doesn't generate comment indeed but it upload it and should be used the other workflow I metion at start.

The PR workflow:

pr

The main workflow:

main
@awxiaoxian2020 awxiaoxian2020 mentioned this issue Mar 23, 2023
Closed
@awxiaoxian2020
Copy link
Contributor Author

awxiaoxian2020 commented Mar 23, 2023 •
edited
Loading

react.dev/.github/workflows/analyze_comment.yml

Line 13 in b9af9f5

${{ github.event.workflow_run.event == 'pull_request' &&

It seems not including push and we shouldn't but how we handle with this noisy notification?

@awxiaoxian2020 awxiaoxian2020 changed the title [GH workflow] analyze_bundle.txt missed on main branch or the workflow set incorrectly [GH workflow] update analyze.yml to avoid noisy notification Mar 23, 2023
@gaearon
Copy link
Member

gaearon commented Mar 23, 2023

If you have a fix in mind, please send it!

@awxiaoxian2020
Copy link
Contributor Author

@gaearon The point is that do we need analyze comment on main branch?

@harish-sethuraman
Copy link
Collaborator

Yea it is better to have. Because sometimes commits land directly into main rather than PR.

@awxiaoxian2020
Copy link
Contributor Author

Yea it is better to have. Because sometimes commits land directly into main rather than PR.

We can add conditions, I think. I will look into it on the weekend.

awxiaoxian2020 added a commit to reactjs/zh-hans.react.dev that referenced this issue Mar 31, 2023
@awxiaoxian2020
chore: fix workflow (#1112)
7e10cd5 
just a test for running on main branch. related to reactjs/react.dev#5801. maybe revert at last.
This was referenced Apr 1, 2023
Closed
Closed
@awxiaoxian2020 awxiaoxian2020 linked a pull request Jul 25, 2023 that will close this issue
Open
@alinkedd alinkedd mentioned this issue Feb 20, 2024
Closed
@alinkedd alinkedd linked a pull request Mar 3, 2025 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@gaearon @harish-sethuraman @awxiaoxian2020