SHA-245: Fix dist path #547
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 WalkthroughWalkthroughThe pull request modifies the GitHub Actions workflow for publishing releases. It updates the path for downloading the Changes
Assessment against linked issues
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range comments (3)
.github/workflows/publish_release.yml (3)
Line range hint
11-11: Fix the conditional check for merged pull requestsThe condition
github.event.pull_request.mergedwon't work in aworkflow_runtrigger context because the pull request information isn't directly available. This could prevent the job from executing as intended.Consider using the workflow run context instead:
- if: ${{ github.event.workflow_run.conclusion == 'success' && github.event.workflow_run.event == 'pull_request' && github.event.pull_request.merged == true }} + if: ${{ github.event.workflow_run.conclusion == 'success' && github.event.workflow_run.event == 'pull_request' }}Note: The merged status check should be handled in the triggering workflow ("Prepare Release Assets") instead.
Line range hint
28-57: Enhance release script robustness and error handlingThe release asset upload script could benefit from several improvements:
- Add error handling for missing releases:
let draftRelease = releases.data.find(release => release.tag_name === releaseTag && release.draft === true); + if (!draftRelease) { + throw new Error(`No draft release found with tag ${releaseTag}`); + }
- Add error handling for file existence:
+ const fs = require('fs'); + if (!fs.existsSync(assetPath)) { + throw new Error(`Asset not found at path: ${assetPath}`); + } - const data = require('fs').readFileSync(assetPath); + const data = fs.readFileSync(assetPath);
- Include content-type in upload:
await github.rest.repos.uploadReleaseAsset({ owner, repo, release_id: releaseId, name: assetName, data, + headers: { + 'content-type': assetContentType, + }, });
Line range hint
1-57: Consider splitting the workflow into smaller, focused jobsThe current workflow combines release management and asset upload into a single job. Consider:
Split into separate jobs:
- Release management (finding/updating release)
- Asset validation (checking paths/files)
- Asset upload
Add job outputs and dependencies to share data between jobs
Add workflow-level environment variables for paths and asset names
This would improve maintainability and make the workflow easier to debug.
What's Changed
Summary by CodeRabbit