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.

Sign up for GitHub

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

chore: update release action #2459

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6a4dc84
chore: update release action
MrKou47 Dec 12, 2024
f47a9c9
fix: yaml indent error
MrKou47 Dec 23, 2024
521ad63
fix: syntax error
MrKou47 Dec 23, 2024
bdc2c2c
chore: use working-directory
MrKou47 Dec 23, 2024
a50e6c0
chore: display more install info
MrKou47 Dec 23, 2024
12fd732
chore: debug
MrKou47 Dec 23, 2024
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
98 changes: 90 additions & 8 deletions .github/workflows/release.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ jobs:
permissions:
contents: write
id-token: write

steps:
- 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 +34,94 @@ jobs:
registry-url: https://registry.npmjs.org/
cache: pnpm

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

- name: Release current monorepo
uses: galacean/[email protected]
- 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 toolkit
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
MrKou47 marked this conversation as resolved.
Show resolved Hide resolved

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

Copy link

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"

- 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@tag
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 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.

MrKou47 marked this conversation as resolved.
Show resolved Hide resolved
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}}
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"
}

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

MrKou47 marked this conversation as resolved.
Show resolved Hide resolved
- 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}}
MrKou47 marked this conversation as resolved.
Show resolved Hide resolved
Loading