fix(server): fix command failed #36
Conversation
WalkthroughThe pull request modifies the workflow configuration for Changes
Suggested reviewers
Poem
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 (
|
✅ Deploy Preview for reearth-classic canceled.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
.github/workflows/build_server.yml (2)
Line range hint
94-114: Simplify conditional logic for better maintainabilityThe current implementation of version and tag logic, while functional, could be simplified for better maintainability.
Consider refactoring the script to use functions and clearer variable names:
- if [[ -n $TAG ]]; then - PLATFORMS=linux/amd64,linux/arm64 - VERSION=$TAG - TAGS=$IMAGE_NAME:$TAG - if [[ ! $TAG =~ '-' ]]; then - TAGS+=,${IMAGE_NAME}:${TAG%.*} - TAGS+=,${IMAGE_NAME}:${TAG%%.*} - TAGS+=,${IMAGE_NAME}:latest - fi - else - PLATFORMS=linux/amd64 - VERSION=$SHA - TAGS=$IMAGE_NAME:$NAME - fi + # Function to generate tag list + generate_tags() { + local base_tag=$1 + local tags="$IMAGE_NAME:$base_tag" + + # Add additional tags for stable releases + if [[ ! $base_tag =~ '-' ]]; then + tags+=",${IMAGE_NAME}:${base_tag%.*}" # Major.Minor + tags+=",${IMAGE_NAME}:${base_tag%%.*}" # Major + tags+=",${IMAGE_NAME}:latest" + fi + echo "$tags" + } + + # Set build configuration + PLATFORMS=$([ -n "$TAG" ] && echo "linux/amd64,linux/arm64" || echo "linux/amd64") + VERSION=${TAG:-$SHA} + TAGS=$([ -n "$TAG" ] && generate_tags "$TAG" || echo "$IMAGE_NAME:$NAME")This refactoring:
- Improves readability with a dedicated function for tag generation
- Makes the platform selection more concise
- Maintains the same functionality while being easier to maintain
Line range hint
115-121: Simplify complex version selection logicThe current version selection using nested ternary operators is hard to read and maintain.
Consider simplifying the version selection:
- version: tags/${{ github.event.inputs.name && github.event.inputs.name != 'blank' && github.event.inputs.name || github.event.inputs.new_tag }} - file: reearth-web_${{ github.event.inputs.name && github.event.inputs.name != 'blank' && github.event.inputs.name || github.event.inputs.new_tag }}.tar.gz + env: + RELEASE_VERSION: ${{ github.event.inputs.name != 'blank' && github.event.inputs.name || github.event.inputs.new_tag }} + version: tags/${{ env.RELEASE_VERSION }} + file: reearth-web_${{ env.RELEASE_VERSION }}.tar.gzThis change:
- Reduces duplication
- Improves readability
- Makes debugging easier
- Reduces the chance of inconsistencies
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/build_server.yml(1 hunks)
🔇 Additional comments (1)
.github/workflows/build_server.yml (1)
43-43: Verify impact of removing --rm-dist flag
While removing the --rm-dist flag may fix the immediate build failure, this could lead to:
- Mixing of old and new artifacts in the
distdirectory - Accumulation of artifacts over multiple builds
- Potential confusion about artifact origins
Let's verify the impact:
Consider adding an explicit cleanup step before the GoReleaser action:
- name: Clean dist directory
run: rm -rf server/dist
if: success() || failure() # Run on both success and failure✅ Verification successful
Removing --rm-dist flag is safe in this context
The workflow already handles artifacts appropriately:
- Build artifacts are stored in a specific path
server/dist/reearth_*.* - The workflow includes a verification step that lists dist contents (
ls -l server/dist) - GoReleaser is configured to use a specific distribution and tag
The removal of --rm-dist flag won't cause issues because:
- Each workflow run starts with a fresh environment
- Artifacts are properly scoped to the
server/distdirectory - The build process is isolated per workflow execution
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if there are any existing issues or discussions about --rm-dist
rg -i "rm-dist|clean dist" .github/
# Check for any cleanup steps in other workflows
fd -t f . .github/workflows/ --exec grep -l "clean" {} \;
Length of output: 98
Script:
#!/bin/bash
# Let's check the goreleaser configuration and related files
fd -t f "\.ya?ml$" . -E ".git" --exec grep -l "goreleaser|dist" {} \;
# Check the content of goreleaser config if it exists
fd "\.goreleaser\.ya?ml$" . -E ".git" --exec cat {} \;
# Look for any cleanup or dist-related tasks in workflow files
rg "(?i)(clean|dist|goreleaser)" .github/workflows/
Length of output: 807
Overview
fix command failed
What I've done
The build is failing, so I will remove the command options.
What I haven't done
How I tested
Which point I want you to review particularly
Memo
Summary by CodeRabbit