-
Notifications
You must be signed in to change notification settings - Fork 7.8k
Update and fix gh actions #6651
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Size changes📦 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! 🙌 |
802097f to
ecaad6a
Compare
ecaad6a to
1fded7f
Compare
|
@gaearon @harish-sethuraman could you please help to find someone to review this PR regarding GH actions? It would be great to have them fixed. duplicates #6179 (comment) |
1fded7f to
ef622a3
Compare
ef622a3 to
b985ceb
Compare
b985ceb to
60e7311
Compare
|
@poteto hi |
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.
thanks for the contribution! rather than update the existing external action, i would rather we make use of the github ones. could you help rewrite them to use it instead?
|
|
||
| - name: Download base branch bundle stats | ||
| uses: dawidd6/action-download-artifact@v2 | ||
| uses: dawidd6/action-download-artifact@v6 |
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 really need an external action for this? could we rewrite this to use https://github.com/actions/toolkit/tree/main/packages/artifact instead?
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.
Sorry for the delayed response
From what I understand, previous versions of actions/download-artifact didn't support retrieving an artifact from a different workflow, but dawidd6/action-download-artifact enabled this via a REST API for downloading.
It now looks like actions/download-artifact@v4 officially supports this feature, so it should be easier to implement that using only the official package.
I'll try to work on this over the weekend
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.
@poteto I've completely forgot about this one :) I've moved the fixes to #7667 since they're small and ready to go.
However, I'm not available to work on the improvement of switching from dawidd6/action-download-artifact to the latest version of actions/download-artifact, perhaps it could be converted into a separate issue
|
Close in favor of #7667 |
except for the). Close Update and fix gh actions #6651upload-artifactupload-artifactwas bumped to v3, as I'm not sure about the required unique artifact names in v4, though v3 still uses node16. Also close [Bug]:analyzeworkflow fails because of deprecated version ofactions/upload-artifact#7148Upload analysis commentstep)Analyze Bundle (Comment)action failure on the default branch by rewriting the condition, as GitHub's runner evaluation is tricky and sometimes provides false positives. Close [GH workflow] updateanalyze.ymlto avoid noisy notification #5801NOTE: analyze-comment depends on
workflow_runand thus will be used only after merging updates into the default branch. This is why the deprecation warning is still present (https://github.com/reactjs/react.dev/actions/runs/7980542658). To check the proposed fix in a condition, you may look over the same fix in my test repo and then dependant workflow respecting that condition.Thorough review is welcomed, as I'm no GitHub Actions professional :)
UPD
I added folded block to
ifcondition without expression to shorten the line, and it seems to work file. Links to my latest test results are updated.UPD 2
#7277 has already updated most of the actions, so it closes #5961 by updating dawidd6/action-download-artifact at last
UPD 3
I've moved fixes to #7667.
dawidd6/action-download-artifactshould be reworked as part of a different issue