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.

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

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 84 additions & 3 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ jobs:
contents: write
id-token: write
steps:
- uses: actions/checkout@v4
- name: Checkout Engine repo
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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.

Suggested change
- 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)

uses: actions/checkout@v4
with:
fetch-depth: 0

Expand All @@ -32,14 +33,94 @@ jobs:
registry-url: https://registry.npmjs.org/
cache: pnpm

- name: Build
- name: Build Engine
run: pnpm b:all

- name: Release current monorepo
- name: Cache Engine Package
uses: actions/cache@v3
with:
path: ./packages/galacean
key: ${{ runner.os }}-engine-${{ github.sha }}

Comment on lines +39 to +44
Copy link

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

  1. The actions/cache@v3 action should be updated to v4 for better compatibility with current runners.
  2. 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.

Suggested change
- 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)

- 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
Comment on lines +45 to +49
Copy link

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.


- 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 lottie
pnpm install
Copy link
Member

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

pnpm link ../packages/galacean
pnpm build

- name: Install and Link Engine for Spine
run: |
cd spine
pnpm install
pnpm link ../packages/galacean
pnpm build

- name: Ensure Dist Directory Exists
run: mkdir -p ${{ github.workspace }}/platform-adapter/dist

# Create a mock package.json to specify the path and version of the adapter build result when syncing with the CDN later.
# name is set to @galacean/engine-platform-adapter-release to avoid conflicts with the real package.json
# version is set to the version of the engine package
- name: Create package.json in platform-adapter
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
Comment on lines +86 to +88
Copy link

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.

Suggested change
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: Bundle polyfill and engine
uses: galacean/platform-adapter@main
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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.

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"
}
Comment on lines +90 to +110
Copy link

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

  1. Using @main tag is risky as it can change unexpectedly
  2. 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: Release engine packages and Sync to CDN
Copy link

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

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}}

- name: Sync Platform Adapter to CDN
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}}
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
"release": "bumpp -r"
},
"devDependencies": {
"@actions/core": "^1.11.1",
"@commitlint/cli": "^11.0.0",
"@commitlint/config-conventional": "^11.0.0",
"@rollup/plugin-commonjs": "^17.0.0",
Expand Down
Empty file added packages/core/src/audio.ts
Empty file.
48 changes: 46 additions & 2 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Empty file added test.js
Empty file.
Loading