-
-
Notifications
You must be signed in to change notification settings - Fork 108
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: updated to actions/upload-artifact v4 #574
Conversation
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.
❌ Changes requested. Reviewed everything up to 326e564 in 1 minute and 26 seconds
More details
- Looked at
53
lines of code in2
files - Skipped
0
files when reviewing. - Skipped posting
0
drafted comments based on config settings.
Workflow ID: wflow_NGv07vHFqO16fU8a
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
⌛ 9 days left in your free trial, upgrade for $20/seat/month or contact us.
.github/workflows/comment.yml
Outdated
}); | ||
var fs = require('fs'); | ||
fs.writeFileSync(`${{github.workspace}}/${artifact.name}.zip`, Buffer.from(download.data)); | ||
} | ||
|
||
- run: unzip -d screenshots screenshots.zip |
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.
The unzip command attempts to unzip a file named 'screenshots.zip', but the artifacts are now dynamically named and saved as individual zip files. This will cause the unzip step to fail. Consider updating this step to handle multiple zip files or adjust the naming convention to ensure the correct file is targeted.
- run: unzip -d screenshots screenshots.zip | |
# Adjust this command to handle the new naming convention or process multiple zip files. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #574 +/- ##
=======================================
Coverage 26.15% 26.15%
=======================================
Files 27 27
Lines 1629 1629
Branches 285 285
=======================================
Hits 426 426
+ Misses 1177 1145 -32
- Partials 26 58 +32 ☔ View full report in Codecov by Sentry. |
Needs to be merged before changes to the comment workflow are used, unlike: https://github.com/ActivityWatch/aw-webui/actions/runs/9044104886 |
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.
👍 Looks good to me! Incremental review on 57c378b in 1 minute and 54 seconds
More details
- Looked at
30
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. .github/workflows/comment.yml:53
- Draft comment:
Consider using asynchronous methods such aschild_process.exec
or promises withutil.promisify
to handle the unzipping process. This will prevent blocking the Node.js event loop and potentially improve the performance of the workflow. - Reason this comment was not posted:
Confidence of 0% on close inspection, compared to threshold of 50%.
Workflow ID: wflow_I3WujMcskAmmqrw1
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
⌛ 9 days left in your free trial, upgrade for $20/seat/month or contact us.
Fixes #560
Works around actions/upload-artifact#478
Summary:
Updated GitHub Actions workflows to use
actions/upload-artifact@v4
, enhancing artifact management and naming in CI processes.Key points:
actions/upload-artifact
tov4
in/github/workflows/comment.yml
and/github/workflows/nodejs.yml
.comment.yml
to include unzipping and cleanup within the script.nodejs.yml
based on matrix variables.Generated with ❤️ by ellipsis.dev