-
-
Notifications
You must be signed in to change notification settings - Fork 309
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
chore: update release action #2459
Conversation
Caution Review failedThe pull request is closed. WalkthroughThe changes in the pull request primarily focus on enhancing the release workflow defined in Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
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.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (3)
.github/workflows/release.yml (3)
40-44
: Optimize cache configurationConsider adding a fallback cache key to improve cache hit rates. This allows using a previous cache when the exact commit SHA doesn't match.
uses: actions/cache@v3 with: path: ./packages/galacean - key: ${{ runner.os }}-engine-${{ github.sha }} + key: ${{ runner.os }}-engine-${{ github.sha }} + restore-keys: | + ${{ runner.os }}-engine-
80-82
: Add error handling for version extractionThe version extraction process should handle potential errors and validate the version format.
run: | - VERSION=$(jq -r '.version' ${{ github.workspace }}/packages/galacean/package.json) + VERSION=$(jq -r '.version' ${{ github.workspace }}/packages/galacean/package.json) + if [[ ! $VERSION =~ ^[0-9]+\.[0-9]+\.[0-9]+(-[0-9A-Za-z-]+(\.[0-9A-Za-z-]+)*)?(\+[0-9A-Za-z-]+)?$ ]]; then + echo "Error: Invalid version format: $VERSION" + exit 1 + fi echo "{\"name\": \"@galacean/engine-platform-adapter-release\", \"version\": \"$VERSION\"}" > ${{ github.workspace }}/platform-adapter/package.json
Node.js version requirement should be lowered to LTS
The specified Node.js version requirement of '>=22.6.0' is unnecessarily high. Based on the codebase analysis:
- No package.json files specify any Node.js engine requirements
- The codebase primarily consists of TypeScript/JavaScript files using standard ES modules syntax
- The build tools used (rollup, typescript, etc.) are all compatible with Node.js LTS versions
Recommend changing to Node.js 20.x (Current LTS) as it provides all necessary features while being production-ready.
🔗 Analysis chain
Line range hint
31-33
: Verify Node.js version requirementThe specified Node.js version requirement of '>=22.6.0' seems unusually high. Node.js 22 is currently in pre-release status and not recommended for production use. Consider using the current LTS version (20.x) instead.
- node-version: '>=22.6.0' + node-version: '20.x'🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if any dependencies require Node.js 22 fd package.json --exec jq -r '.engines.node' {} 2>/dev/nullLength of output: 124
Script:
#!/bin/bash # Check for any Node.js version requirements in package.json files fd package.json --exec cat {} | grep -A 5 '"engines"' 2>/dev/null # Also check for any Node.js-specific features or syntax in JavaScript/TypeScript files rg -l --type-add 'ts:*.{ts,tsx}' --type-add 'js:*.{js,jsx,mjs}' --type ts,js '(?:import\s+.*\s+from|export\s+.*\s+from|import\s*\(|import\.meta)'Length of output: 235
Script:
#!/bin/bash # Check for any Node.js version requirements in package.json files fd package.json --exec cat {} # Check for modern JavaScript features in source files rg -t js -t ts "(?:import\s+.*\s+from|export\s+.*\s+from|import\s*\(|import\.meta)" -lLength of output: 47316
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/release.yml
(2 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/release.yml
83-83: could not parse as YAML: yaml: line 83: did not find expected key
(syntax-check)
🪛 yamllint (1.35.1)
.github/workflows/release.yml
[error] 106-106: syntax error: expected , but found '-'
(syntax)
.github/workflows/release.yml
Outdated
- name: Checkout Sub-repositories | ||
run: | | ||
git clone https://github.com/galacean/engine-toolkit.git | ||
git clone https://github.com/galacean/engine-lottie.git | ||
git clone https://github.com/galacean/engine-spine.git | ||
|
||
- name: Install and Link Engine for Toolkit | ||
run: | | ||
cd toolkit | ||
pnpm install | ||
pnpm link ../packages/galacean | ||
pnpm b:all | ||
|
||
- name: Install and Link Engine for Lottie | ||
run: | | ||
cd toolkit | ||
pnpm install | ||
pnpm link ../packages/galacean | ||
pnpm build | ||
|
||
- name: Install and Link Engine for Spine | ||
run: | | ||
cd toolkit | ||
pnpm install | ||
pnpm link ../packages/galacean | ||
pnpm build | ||
|
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.
🛠️ Refactor suggestion
Refactor repeated setup logic
The setup logic for sub-repositories is duplicated. Consider creating a reusable composite action or shell script.
Example implementation:
- name: Setup Sub-repository
run: |
setup_repo() {
local repo_name=$1
cd $repo_name
pnpm install
pnpm link ../packages/galacean
pnpm ${2:-build}
cd ..
}
setup_repo "engine-toolkit" "b:all"
setup_repo "engine-lottie"
setup_repo "engine-spine"
.github/workflows/release.yml
Outdated
- name: Bundle polyfill and engine | ||
uses: galacean/platform-adapter@tag |
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.
Fix invalid action reference
The action reference uses: galacean/platform-adapter@tag
appears to use a literal "tag" instead of a specific version or ref.
- uses: galacean/platform-adapter@tag
+ uses: galacean/platform-adapter@v1 # Replace with actual version
Committable suggestion skipped: line range outside the PR's diff.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev/1.4 #2459 +/- ##
===========================================
+ Coverage 68.33% 68.48% +0.14%
===========================================
Files 908 921 +13
Lines 94377 95425 +1048
Branches 7926 8119 +193
===========================================
+ Hits 64492 65348 +856
- Misses 29635 29825 +190
- Partials 250 252 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
- name: Install and Link Engine for Lottie | ||
run: | | ||
cd toolkit | ||
pnpm install |
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.
Here is a problem: the version numbers of these packages do not match the engine
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.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
.github/workflows/release.yml (1)
Line range hint
31-33
: Specify exact Node.js version instead of using a rangeUsing
>=22.6.0
could lead to unexpected behavior if a new Node.js version introduces breaking changes. Consider using a specific LTS version.with: - node-version: '>=22.6.0' + node-version: '20.10.0' # Latest LTS version registry-url: https://registry.npmjs.org/🧰 Tools
🪛 actionlint (1.7.4)
19-19: step must run script with "run" section or run action with "uses" section
(syntax-check)
♻️ Duplicate comments (1)
.github/workflows/release.yml (1)
54-54
:⚠️ Potential issueFix incorrect directory paths
The
cd
commands are using wrong directory names.- cd toolkit + cd engine-toolkit - cd lottie + cd engine-lottie - cd spine + cd engine-spineAlso applies to: 61-61, 68-68
🧹 Nitpick comments (2)
.github/workflows/release.yml (2)
40-44
: Optimize cache configurationThe current cache configuration could be improved:
- Include package.json hash in the cache key for more precise caching
- Add restore-keys for fallback cache hits
with: path: ./packages/galacean - key: ${{ runner.os }}-engine-${{ github.sha }} + key: ${{ runner.os }}-engine-${{ hashFiles('**/package.json') }}-${{ github.sha }} + restore-keys: | + ${{ runner.os }}-engine-${{ hashFiles('**/package.json') }}- + ${{ runner.os }}-engine-🧰 Tools
🪛 actionlint (1.7.4)
41-41: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
106-127
: Optimize environment variables configurationEnvironment variables are duplicated between steps. Move common variables to the job level.
jobs: release: + env: + NODE_AUTH_TOKEN: ${{secrets.NPM_TOKEN}} + NPM_CONFIG_PROVENANCE: true + OASISBE_UPLOAD_URL: https://oasisbe.alipay.com/api/file/no-auth/crypto/upload + OASISBE_REQUEST_HEADER: ${{secrets.OASISBE_REQUEST_HEADER}} + OASISBE_PUBLIC_KEY: ${{secrets.OASISBE_PUBLIC_KEY}}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/release.yml
(2 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/release.yml
19-19: step must run script with "run" section or run action with "uses" section
(syntax-check)
41-41: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🔇 Additional comments (1)
.github/workflows/release.yml (1)
52-71
: Refactor repeated sub-repository setup logic
The setup logic is duplicated across three sub-repositories. Consider creating a reusable function.
+ - name: Setup Sub-repositories
+ run: |
+ setup_repo() {
+ local repo_name=$1
+ cd $repo_name
+ pnpm install
+ pnpm link ../packages/galacean
+ pnpm ${2:-build}
+ cd ..
+ }
+
+ setup_repo "engine-toolkit" "b:all"
+ setup_repo "engine-lottie"
+ setup_repo "engine-spine"
steps: | ||
- name: Checkout Engine repo |
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.
Fix YAML syntax error in checkout step
The step definition has incorrect syntax. The uses
key should be under the name
key.
- - name: Checkout Engine repo
- - uses: actions/checkout@v4
+ - name: Checkout Engine repo
+ uses: actions/checkout@v4
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
- name: Checkout Engine repo | |
- name: Checkout Engine repo | |
uses: actions/checkout@v4 |
🧰 Tools
🪛 actionlint (1.7.4)
19-19: step must run script with "run" section or run action with "uses" section
(syntax-check)
echo "{\"name\": \"@galacean/engine-platform-adapter-release\", \"version\": \"$VERSION\"}" > ${{ github.workspace }}/platform-adapter/package.json | ||
|
||
- name: Bundle polyfill and engine | ||
uses: galacean/platform-adapter@main |
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.
Use specific version for platform-adapter action
Using the main
branch is risky as it can change unexpectedly. Pin to a specific version or commit SHA.
- uses: galacean/platform-adapter@main
+ uses: galacean/[email protected] # Replace with actual version
Committable suggestion skipped: line range outside the PR's diff.
"outputDir": "${{ github.workspace }}/platform-adapter/dist" | ||
} | ||
|
||
- name: Release engine packages and Sync to CDN |
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.
💡 Codebase verification
Update the galacean/publish action to v0.2.2
The current workflow uses galacean/[email protected]
, but a newer version v0.2.2 is available. Consider updating the action version to use the latest stable release.
🔗 Analysis chain
Verify galacean/publish action version
The action galacean/[email protected]
is used in both release steps. Please verify if this is the latest stable version.
Also applies to: 115-115
Let me try another approach to verify the action version.
Let me try one more approach to check the tags directly.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check latest version and release history of galacean/publish action
# Check latest release
gh api repos/galacean/publish/releases/latest --jq '.tag_name'
# List recent releases
gh api repos/galacean/publish/releases --jq '.[].tag_name'
Length of output: 278
Script:
#!/bin/bash
# Check the repository and its contents directly
gh api repos/galacean/publish/contents/.github/workflows --jq '.[].name'
# Try to get the repository information
gh api repos/galacean/publish
Length of output: 16042
Script:
#!/bin/bash
# Check tags in the repository
gh api repos/galacean/publish/git/refs/tags --jq '.[].ref'
# Try to check the action.yml file in the repository
gh api repos/galacean/publish/contents/action.yml --jq '.content' | base64 -d
Length of output: 299
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.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
.github/workflows/release.yml (1)
Line range hint
31-33
: Fix Node.js version constraintUsing
>=22.6.0
for Node.js version is risky as it may lead to unexpected behavior with future Node.js releases. Instead, specify an exact version or a more constrained range.uses: actions/setup-node@v4 with: - node-version: '>=22.6.0' + node-version: '22.x' registry-url: https://registry.npmjs.org/ cache: pnpm🧰 Tools
🪛 actionlint (1.7.4)
40-40: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
♻️ Duplicate comments (3)
.github/workflows/release.yml (3)
51-71
:⚠️ Potential issueFix incorrect directory paths and reduce duplication
- The
cd
commands use incorrect directory names that don't match the cloned repositories.- The installation steps are duplicated across sub-repositories.
- name: Install and Link Engine for Toolkit run: | - cd toolkit + cd engine-toolkit pnpm install pnpm link ../packages/galacean pnpm b:all - name: Install and Link Engine for Lottie run: | - cd lottie + cd engine-lottie pnpm install pnpm link ../packages/galacean pnpm build - name: Install and Link Engine for Spine run: | - cd spine + cd engine-spine pnpm install pnpm link ../packages/galacean pnpm buildConsider refactoring the repeated setup logic into a reusable shell function:
- name: Install and Link Sub-repositories run: | setup_repo() { local repo_name=$1 local build_cmd=${2:-build} cd $repo_name pnpm install pnpm link ../packages/galacean pnpm $build_cmd cd .. } setup_repo "engine-toolkit" "b:all" setup_repo "engine-lottie" setup_repo "engine-spine"
83-104
:⚠️ Potential issueFix action version and add path validation
- Using
@main
for the platform-adapter action is risky as it can change unexpectedly.- The bundle paths should be validated before use.
- uses: galacean/platform-adapter@main + uses: galacean/[email protected] # Replace with actual version env: ADAPTER_BUNDLE_SETTINGS: | { "polyfill": true, "engine": [ "${{ github.workspace }}/packages/galacean/dist/module.js", "${{ github.workspace }}/packages/xr/dist/module.js", "${{ github.workspace }}/packages/shader-lab/dist/module.js", "${{ github.workspace }}/packages/physics-lite/dist/module.js", "${{ github.workspace }}/packages/physics-physx/dist/module.js", "${{ github.workspace }}/engine-lottie/dist/module.js", "${{ github.workspace }}/engine-spine/dist/module.js", "${{ github.workspace }}/engine-toolkit/galacean-engine-toolkit/dist/module.js" ], "jsWASMLoader": [ "${{ github.workspace }}/packages/physics-physx/libs/physx.release.js" ], "outputDir": "${{ github.workspace }}/platform-adapter/dist" }Add a validation step before bundling:
- name: Validate Bundle Paths run: | validate_path() { if [ ! -f "$1" ]; then echo "Error: File not found: $1" exit 1 fi } validate_path "${{ github.workspace }}/packages/galacean/dist/module.js" validate_path "${{ github.workspace }}/packages/xr/dist/module.js" # Add validation for other paths
105-126
: 🛠️ Refactor suggestionOptimize environment configuration and update action version
- Environment variables are duplicated between steps
- The action version should be updated to v0.2.2
Move common environment variables to job level:
jobs: release: env: NODE_AUTH_TOKEN: ${{secrets.NPM_TOKEN}} NPM_CONFIG_PROVENANCE: true OASISBE_UPLOAD_URL: https://oasisbe.alipay.com/api/file/no-auth/crypto/upload OASISBE_REQUEST_HEADER: ${{secrets.OASISBE_REQUEST_HEADER}} OASISBE_PUBLIC_KEY: ${{secrets.OASISBE_PUBLIC_KEY}}Update the action version:
- uses: galacean/[email protected] + uses: galacean/[email protected]
🧹 Nitpick comments (2)
.github/workflows/release.yml (2)
39-44
: Enhance cache configurationThe cache configuration could be improved:
- Add
restore-keys
for fallback cache hits- Include pnpm's lockfile hash in the cache key for more precise caching
uses: actions/cache@v3 with: path: ./packages/galacean - key: ${{ runner.os }}-engine-${{ github.sha }} + key: ${{ runner.os }}-engine-${{ hashFiles('**/pnpm-lock.yaml') }}-${{ github.sha }} + restore-keys: | + ${{ runner.os }}-engine-${{ hashFiles('**/pnpm-lock.yaml') }}- + ${{ runner.os }}-engine-🧰 Tools
🪛 actionlint (1.7.4)
40-40: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
79-81
: Add error handling for version extractionThe version extraction could fail silently if the package.json is malformed or missing.
run: | + if [ ! -f "${{ github.workspace }}/packages/galacean/package.json" ]; then + echo "Error: package.json not found" + exit 1 + fi VERSION=$(jq -r '.version' ${{ github.workspace }}/packages/galacean/package.json) + if [ -z "$VERSION" ]; then + echo "Error: Failed to extract version" + exit 1 + fi echo "{\"name\": \"@galacean/engine-platform-adapter-release\", \"version\": \"$VERSION\"}" > ${{ github.workspace }}/platform-adapter/package.json
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
.github/workflows/release.yml
(2 hunks)package.json
(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/release.yml
40-40: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🔇 Additional comments (1)
package.json (1)
27-27
: LGTM: Addition of @actions/core dependency
The addition of @actions/core
is appropriate for enhancing GitHub Actions workflow functionality.
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.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/release.yml (1)
86-103
: Add path validation for bundle settingsThe bundle configuration assumes all specified paths exist. Consider adding validation to ensure required files are present before bundling.
- name: Validate bundle paths run: | for path in \ "${{ github.workspace }}/packages/galacean/dist/module.js" \ "${{ github.workspace }}/packages/xr/dist/module.js" \ # ... add other paths do if [ ! -f "$path" ]; then echo "Error: Required file not found: $path" exit 1 fi done
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/release.yml
(2 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/release.yml
40-40: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🔇 Additional comments (4)
.github/workflows/release.yml (4)
51-71
: Refactor repeated setup logic
The setup logic for sub-repositories is duplicated.
84-84
: Use specific version for platform-adapter action
Using the main
branch is risky as it can change unexpectedly.
105-126
: Optimize environment variable configuration
The environment variables are duplicated between the two publish steps. Consider moving them to the job level.
jobs:
release:
runs-on: ubuntu-latest
permissions:
contents: write
id-token: write
+ env:
+ NODE_AUTH_TOKEN: ${{secrets.NPM_TOKEN}}
+ NPM_CONFIG_PROVENANCE: true
+ OASISBE_UPLOAD_URL: https://oasisbe.alipay.com/api/file/no-auth/crypto/upload
+ OASISBE_REQUEST_HEADER: ${{secrets.OASISBE_REQUEST_HEADER}}
+ OASISBE_PUBLIC_KEY: ${{secrets.OASISBE_PUBLIC_KEY}}
Additionally, consider updating to the latest version of the publish action.
Line range hint 31-34
: Verify Node.js version requirement
The specified Node.js version requirement >=22.6.0
seems unusually high, as Node.js 22 is still in development. This might cause compatibility issues.
Consider using an LTS version like 20.x:
- node-version: '>=22.6.0'
+ node-version: '20.x'
🧰 Tools
🪛 actionlint (1.7.4)
40-40: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
run: | | ||
VERSION=$(jq -r '.version' ${{ github.workspace }}/packages/galacean/package.json) | ||
echo "{\"name\": \"@galacean/engine-platform-adapter-release\", \"version\": \"$VERSION\"}" > ${{ github.workspace }}/platform-adapter/package.json |
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.
🛠️ Refactor suggestion
Add error handling for version extraction
The version extraction using jq
lacks error handling. If the package.json is malformed or the version field is missing, this could fail silently.
- VERSION=$(jq -r '.version' ${{ github.workspace }}/packages/galacean/package.json)
+ VERSION=$(jq -r '.version' ${{ github.workspace }}/packages/galacean/package.json)
+ if [ -z "$VERSION" ]; then
+ echo "Error: Failed to extract version from package.json"
+ exit 1
+ fi
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
run: | | |
VERSION=$(jq -r '.version' ${{ github.workspace }}/packages/galacean/package.json) | |
echo "{\"name\": \"@galacean/engine-platform-adapter-release\", \"version\": \"$VERSION\"}" > ${{ github.workspace }}/platform-adapter/package.json | |
run: | | |
VERSION=$(jq -r '.version' ${{ github.workspace }}/packages/galacean/package.json) | |
if [ -z "$VERSION" ]; then | |
echo "Error: Failed to extract version from package.json" | |
exit 1 | |
fi | |
echo "{\"name\": \"@galacean/engine-platform-adapter-release\", \"version\": \"$VERSION\"}" > ${{ github.workspace }}/platform-adapter/package.json |
- name: Checkout Sub-repositories | ||
run: | | ||
git clone https://github.com/galacean/engine-toolkit.git | ||
git clone https://github.com/galacean/engine-lottie.git | ||
git clone https://github.com/galacean/engine-spine.git |
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.
🛠️ Refactor suggestion
Add authentication for git operations
Using unauthenticated git clone might hit GitHub's rate limits. Consider using a token for authentication:
- git clone https://github.com/galacean/engine-toolkit.git
+ git clone https://${{ secrets.GITHUB_TOKEN }}@github.com/galacean/engine-toolkit.git
Committable suggestion skipped: line range outside the PR's diff.
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.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
.github/workflows/release.yml (1)
Node.js version 22.6.0+ is too aggressive for production releases
The specified Node.js version requirement (
>=22.6.0
) in the release workflow is concerning because:
- None of the package.json files specify any Node.js engine requirements
- Node.js 22.x is currently in preview/beta status and not recommended for production use
- The latest LTS version is Node.js 20.x, which would be more appropriate for a release workflow
Recommendation: Use Node.js 20.x LTS version (e.g.,
20.x
) in the release workflow for better stability and compatibility.🔗 Analysis chain
Line range hint
31-33
: Verify Node.js version compatibilityThe specified Node.js version requirement (
>=22.6.0
) is quite high and may be in preview/beta status. Consider using an LTS version for better stability in the release workflow.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if any package.json files specify Node.js engine requirements fd -e json -x jq -r '.engines.node // empty' {}Length of output: 110
Script:
#!/bin/bash # Check package.json content directly fd "package.json" --type f --exec cat {}Length of output: 12210
🧰 Tools
🪛 actionlint (1.7.4)
40-40: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
♻️ Duplicate comments (1)
.github/workflows/release.yml (1)
76-82
: 🛠️ Refactor suggestionAdd error handling for version extraction
The version extraction lacks error handling and validation.
- name: Create package.json in platform-adapter run: | + if [ ! -f "${{ github.workspace }}/packages/galacean/package.json" ]; then + echo "Error: package.json not found" + exit 1 + fi VERSION=$(jq -r '.version' ${{ github.workspace }}/packages/galacean/package.json) + if [ -z "$VERSION" ] || [ "$VERSION" = "null" ]; then + echo "Error: Failed to extract version from package.json" + exit 1 + fi echo "{\"name\": \"@galacean/engine-platform-adapter-release\", \"version\": \"$VERSION\"}" > ${{ github.workspace }}/platform-adapter/package.json
🧹 Nitpick comments (1)
.github/workflows/release.yml (1)
106-127
: Optimize workflow configuration
- Move common environment variables to job level
- Update galacean/publish action to latest version
release: runs-on: ubuntu-latest permissions: contents: write id-token: write + env: + NODE_AUTH_TOKEN: ${{secrets.NPM_TOKEN}} + NPM_CONFIG_PROVENANCE: true + OASISBE_UPLOAD_URL: https://oasisbe.alipay.com/api/file/no-auth/crypto/upload + OASISBE_REQUEST_HEADER: ${{secrets.OASISBE_REQUEST_HEADER}} + OASISBE_PUBLIC_KEY: ${{secrets.OASISBE_PUBLIC_KEY}} steps: # ... other steps ... - name: Release engine packages and Sync to CDN - uses: galacean/[email protected] - env: - NODE_AUTH_TOKEN: ${{secrets.NPM_TOKEN}} - NPM_CONFIG_PROVENANCE: true - OASISBE_UPLOAD_URL: https://oasisbe.alipay.com/api/file/no-auth/crypto/upload - OASISBE_REQUEST_HEADER: ${{secrets.OASISBE_REQUEST_HEADER}} - OASISBE_PUBLIC_KEY: ${{secrets.OASISBE_PUBLIC_KEY}} + uses: galacean/[email protected] - name: Sync Platform Adapter to CDN - uses: galacean/[email protected] + uses: galacean/[email protected] with: publish: false packages: | platform-adapter - env: - NODE_AUTH_TOKEN: ${{secrets.NPM_TOKEN}} - NPM_CONFIG_PROVENANCE: true - OASISBE_UPLOAD_URL: https://oasisbe.alipay.com/api/file/no-auth/crypto/upload - OASISBE_REQUEST_HEADER: ${{secrets.OASISBE_REQUEST_HEADER}} - OASISBE_PUBLIC_KEY: ${{secrets.OASISBE_PUBLIC_KEY}}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/release.yml
(2 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/release.yml
40-40: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🔇 Additional comments (3)
.github/workflows/release.yml (3)
18-19
: LGTM: Proper repository checkout configuration
The fetch-depth: 0
setting is correct for release workflows as it ensures the full Git history is available.
51-71
: Refactor repeated setup logic
The setup logic for sub-repositories is duplicated. Consider creating a reusable function.
+ - name: Setup Sub-repositories
+ run: |
+ setup_repo() {
+ local repo_name=$1
+ local build_cmd=${2:-build}
+ cd "$repo_name"
+ echo "Setting up $repo_name..."
+ pnpm install
+ pnpm link ../packages/galacean
+ pnpm $build_cmd
+ cd ..
+ }
+
+ setup_repo "engine-toolkit" "b:all"
+ setup_repo "engine-lottie"
+ setup_repo "engine-spine"
- - name: Install and Link Engine and Build for Toolkit
- working-directory: ./engine-toolkit
- run: |
- pnpm install
- pnpm link ../packages/galacean
- pnpm b:all
-
- - name: Install and Link Engine and Build for Lottie
- working-directory: ./engine-lottie
- run: |
- pwd
- pnpm install --dir ${{ github.workspace }}/engine-lottie
- pnpm link ../packages/galacean
- pnpm build
-
- - name: Install and Link Engine and Build for Spine
- working-directory: ./engine-spine
- run: |
- pnpm install
- pnpm link ../packages/galacean
- pnpm build
45-49
: 🛠️ Refactor suggestion
Enhance repository cloning security and reliability
- Use authenticated git clone to avoid rate limits
- Add error handling for clone operations
- name: Checkout Sub-repositories
run: |
- git clone https://github.com/galacean/engine-toolkit.git
- git clone https://github.com/galacean/engine-lottie.git
- git clone https://github.com/galacean/engine-spine.git
+ clone_repo() {
+ local repo=$1
+ echo "Cloning $repo..."
+ git clone "https://${{ secrets.GITHUB_TOKEN }}@github.com/galacean/$repo.git" || {
+ echo "Failed to clone $repo"
+ exit 1
+ }
+ }
+
+ clone_repo "engine-toolkit"
+ clone_repo "engine-lottie"
+ clone_repo "engine-spine"
Likely invalid or redundant comment.
- name: Bundle polyfill and engine | ||
uses: galacean/platform-adapter@main | ||
env: | ||
ADAPTER_BUNDLE_SETTINGS: | | ||
{ | ||
"polyfill": true, | ||
"engine": [ | ||
"${{ github.workspace }}/packages/galacean/dist/module.js", | ||
"${{ github.workspace }}/packages/xr/dist/module.js", | ||
"${{ github.workspace }}/packages/shader-lab/dist/module.js", | ||
"${{ github.workspace }}/packages/physics-lite/dist/module.js", | ||
"${{ github.workspace }}/packages/physics-physx/dist/module.js", | ||
"${{ github.workspace }}/engine-lottie/dist/module.js", | ||
"${{ github.workspace }}/engine-spine/dist/module.js", | ||
"${{ github.workspace }}/engine-toolkit/galacean-engine-toolkit/dist/module.js" | ||
], | ||
"jsWASMLoader": [ | ||
"${{ github.workspace }}/packages/physics-physx/libs/physx.release.js" | ||
], | ||
"outputDir": "${{ github.workspace }}/platform-adapter/dist" | ||
} |
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.
💡 Codebase verification
🛠️ Refactor suggestion
Use specific version tag instead of @main for platform-adapter action
The latest release of the platform-adapter is v0.0.0-alpha.2
, and using @main
tag is indeed risky as it can lead to unexpected behavior when the main branch changes. Replace:
uses: galacean/platform-adapter@main
with:
uses: galacean/[email protected]
Additionally, consider moving the bundle configuration to a separate JSON file in the repository for better maintainability.
🔗 Analysis chain
Verify platform adapter action version and enhance configuration
- Using
@main
tag is risky as it can change unexpectedly - Consider moving the bundle configuration to a separate file for better maintainability
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check latest release of platform-adapter
gh api repos/galacean/platform-adapter/releases/latest --jq '.tag_name'
Length of output: 86
- name: Cache Engine Package | ||
uses: actions/cache@v3 | ||
with: | ||
path: ./packages/galacean | ||
key: ${{ runner.os }}-engine-${{ github.sha }} | ||
|
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.
🛠️ Refactor suggestion
Update cache action version and enhance cache key
- The
actions/cache@v3
action should be updated to v4 for better compatibility with current runners. - Consider adding package.json hash to the cache key for more precise invalidation.
- name: Cache Engine Package
- uses: actions/cache@v3
+ uses: actions/cache@v4
with:
path: ./packages/galacean
- key: ${{ runner.os }}-engine-${{ github.sha }}
+ key: ${{ runner.os }}-engine-${{ github.sha }}-${{ hashFiles('**/package.json') }}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
- name: Cache Engine Package | |
uses: actions/cache@v3 | |
with: | |
path: ./packages/galacean | |
key: ${{ runner.os }}-engine-${{ github.sha }} | |
- name: Cache Engine Package | |
uses: actions/cache@v4 | |
with: | |
path: ./packages/galacean | |
key: ${{ runner.os }}-engine-${{ github.sha }}-${{ hashFiles('**/package.json') }} |
🧰 Tools
🪛 actionlint (1.7.4)
40-40: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
Please check if the PR fulfills these requirements
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
What is the current behavior? (You can also link to an open issue here)
What is the new behavior (if this is a feature change)?
Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
Other information:
Summary by CodeRabbit
New Features
package.json
for the platform adapter.Improvements
pnpm
and creation of a distribution directory.@actions/core
indevDependencies
.