Conversation
Signed-off-by: Samuel K <skevetter@pm.me>
📝 WalkthroughWalkthroughThe PR expands platform support by adding ubuntu-latest, js-latest, and go-latest platform definitions to .actrc, introduces actionlint configuration for self-hosted runners, and restructures CI/CD workflows to separate desktop builds into distinct UI, app, and Flatpak jobs with updated Node v24 tooling. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR refactors the act workflow to enable local testing of desktop and Flatpak builds. The workflow is reorganized to use custom runner labels (js-latest, go-latest) mapped to specific Docker images, streamlining the E2E testing and build process for local development.
Changes:
- Added three new task commands to build UI, desktop app, and Flatpak packages locally using act
- Reorganized act workflow with dedicated jobs for E2E tests, UI build, desktop build, and Flatpak build
- Configured actionlint to recognize custom runner labels
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| Taskfile.yml | Added three new tasks (desktop:act:build:ui, desktop:act:build:app, desktop:act:build:flatpak) for triggering specific act workflow jobs |
| .github/workflows/act.yml | Refactored workflow with streamlined E2E test job, reorganized build-ui job, and added new build-desktop and build-flatpak jobs for comprehensive local testing |
| .github/actionlint.yaml | Added configuration to recognize js-latest as a valid self-hosted runner label |
| .actrc | Added platform mappings for ubuntu-latest, js-latest, and go-latest to specific Docker images |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| desktop:act:build: | ||
| desktop:act:build:ui: | ||
| desc: build desktop ui using act |
There was a problem hiding this comment.
The description uses lowercase 'ui' but UI is typically written in uppercase as it's an acronym for User Interface. For consistency with other task descriptions that capitalize product names and acronyms, consider changing this to 'build desktop UI using act'.
| desc: build desktop ui using act | |
| desc: build desktop UI using act |
| cmd: act workflow_dispatch -W .github/workflows/act.yml -j build-desktop | ||
|
|
||
| desktop:act:build:flatpak: | ||
| desc: build desktop flatpak using act |
There was a problem hiding this comment.
The description uses lowercase 'flatpak' but Flatpak is a product name and is typically capitalized. For consistency with other task descriptions that capitalize product names like 'Tauri' and 'Desktop', consider changing this to 'build desktop Flatpak using act'.
| desc: build desktop flatpak using act | |
| desc: build desktop Flatpak using act |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/act.yml (1)
40-51: Test failures are silently ignored due to|| true.The
|| truesuffix on line 50 causes the job to always succeed regardless of test outcome. While this may be intentional for local act testing, it masks actual failures and could lead to unnoticed regressions if this workflow is used in CI validation.Consider removing
|| trueor making it conditional:Suggested approach
- ${{ github.event.inputs.test_focus == '' && 'task cli:test:e2e' || format('task cli:test:e2e:focus -- "{0}"', github.event.inputs.test_focus) }} || true + ${{ github.event.inputs.test_focus == '' && 'task cli:test:e2e' || format('task cli:test:e2e:focus -- "{0}"', github.event.inputs.test_focus) }}
🧹 Nitpick comments (2)
.github/workflows/act.yml (2)
118-129: Consider more specific file matching in find commands.The
findcommand on line 123 matches all files nameddevpod-*, which could inadvertently move unintended files if the dist directory contains other devpod-prefixed artifacts. Line 129 then expects specificallydevpod-linux-amd64.Consider being more explicit to avoid surprises:
Suggested refinement
- name: setup executable shell: bash working-directory: ./desktop/src-tauri run: | mkdir -p ./bin/ - find "../../dist" -type f -name "devpod-*" -exec mv {} ./bin \; + find "../../dist" -type f -name "devpod-linux-amd64" -exec mv {} ./bin \; find ./bin -type f -name "devpod-*" -exec chmod +x {} \; + + - name: rename binaries for desktop build + shell: bash + working-directory: ./desktop/src-tauri + run: mv ./bin/devpod-linux-amd64 ./bin/devpod-cli-x86_64-unknown-linux-gnu
178-179: Consider shallow clone for efficiency.The shared-modules repository is cloned without depth limiting, which downloads the full history. A shallow clone would be faster and use less disk space.
Suggested improvement
- name: clone shared-modules - run: git clone https://github.com/flathub/shared-modules.git desktop/flatpak/shared-modules + run: git clone --depth 1 https://github.com/flathub/shared-modules.git desktop/flatpak/shared-modules
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.actrc.github/actionlint.yaml.github/workflows/act.ymlTaskfile.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Agent
- GitHub Check: Build CLI Binary on ubuntu-latest
- GitHub Check: Build CLI Binary on windows-latest
- GitHub Check: Build CLI Binary on macos-latest
- GitHub Check: Lint
🔇 Additional comments (7)
.github/actionlint.yaml (1)
1-3: LGTM!The actionlint configuration correctly defines the
js-latestself-hosted runner label, which aligns with the platform definition in.actrcand the workflow'sruns-onconfiguration..actrc (1)
4-6: LGTM!The platform definitions are correctly configured. Note that
go-latestis defined but not currently referenced in the workflow—this may be intentional for future use.Taskfile.yml (1)
170-180: LGTM!The task split into distinct
build:ui,build:app, andbuild:flatpaktargets improves modularity and aligns with the separate workflow jobs. The job IDs correctly reference the corresponding workflow jobs..github/workflows/act.yml (4)
52-82: LGTM!The build-ui job is well-structured with proper linting, formatting, and type checking before the build. Using
corepack enable yarnensures consistent Yarn version management.
140-149: Signing key approach is appropriate for CI test builds.The throwaway signing key generation for test CI builds is a reasonable approach. The hardcoded password
"github-actions"is acceptable here since this is explicitly for non-production test builds and the key is ephemeral.For production releases, ensure a separate workflow with proper secret management is used.
189-206: Version strings are correctly correlated.The hardcoded
DevPod_0.0.0_amd64.debon line 192 correctly matches theVERSION="v0.0.0"on line 202. This coupling is appropriate for PR/test builds.For release builds, consider parameterizing the version through workflow inputs or extracting it from tags to avoid manual synchronization.
207-211: No action required — flatpak-github-actions v6 is compatible with ghcr.io/flathub-infra/flatpak-github-actions:gnome-48.The configuration in the workflow is correct and follows the recommended setup documented in the official flatpak-github-actions repository and GitHub Marketplace examples.
Signed-off-by: Samuel K skevetter@pm.me
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.