Repository navigation
refactor(desktop): pass --skip-platform in workspace watcher - #1691
Conversation
✅ Deploy Preview for devsydev canceled.
|
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Update workspace listing fallback desktop/src/main/watcher.ts, desktop/src/main/__tests__/watcher.test.ts |
The CLI fallback replaces --skip-pro with --skip-platform. The test checks that cli.run receives the updated arguments. |
Priority:
Estimated code review effort: 2 (Simple) | ~5 minutes
Change: Bug fix
Possibly related issues
- Issue 1551 tracks the Pro-to-Platform rename and identifies the desktop watcher flag updated in this change.
Merge Risk: 
Minimal · up to 872dd
-
The workspace watcher uses the supported renamed flag, so workspace listing should continue to work across the daemon fallback path.
Pre-merge checks: 
4 · 
1
-
Failed checks: 1 warning Passed checks: 4 passed-
Full details: Title check
-
Explanation
The title "refactor(desktop): pass --skip-platform in workspace watcher" matches the change. The desktop watcher now runs
workspace list --skip-platforminstead of--skip-pro, and the test asserts the exact argument list. It follows Conventional Commits form and is short, specific, and free of noise. The title labels the change as a refactor, but the PR description reports that the CLI keeps--skip-proas a hidden alias, so the old flag still works and the observable behavior is preserved, which fits the refactor label.
Full details: Docstring Coverage
-
Explanation
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2 files.
-
Full details: Description check
-
Explanation
The description covers most of the template. It has a clear What and why section that leads with the problem: the CLI renamed
--skip-proto--skip-platformin#1639, and the desktop app still sent the old flag. It includes a Tier note, a detailed Validation list, a CodeRabbit CLI result, and a Remaining note. Several template sections are missing or incomplete: theFixes #issue reference is unfilled (it should sayFixes #N,Refs #N, ornone), the Type of change checkboxes are not ticked (the change is a refactor, so that box should be checked), the AI assistance section is absent (the template requires a choice between the two checkboxes), and the Testing section has no checkbox selected. The checklist items formise run cli:lint:ci,mise run cli:test, and the CLA are not addressed, and the Testing section does not state whether an e2e test was added or why none is needed. The description also states that the change is 'Behaviour-preserving' and that it is eligible for auto-merge, which goes beyond what the template asks for.Resolution
Fill in the
Fixes #line with an issue number ornone. Tick the Refactor box under Type of change. Choose one option in the AI assistance section and complete the table if AI tools were used. Complete the Testing section: tick the Desktop change box and name the Playwright e2e test that covers the change, or tick 'No integration test needed' and name the existing spec that covers the watcher poll path. Confirm the checklist items:mise run desktop:checkfor the desktop change, the CLA, and the docs note if needed. Replace the Auto-merge section with a note in the Notes for reviewers section, or remove it.
-
Finishing Touches
-
✨ Simplify code
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
✅ Deploy Preview for images-devsy-sh canceled.
|
|
This pull request does not currently match the merge queue conditions, so it cannot be queued from here. The box comes back if it matches again. |
Summary
desktop/src/main/watcher.ts: workspace poll runsdevsy workspace list --skip-platforminstead of--skip-pro.desktop/src/main/__tests__/watcher.test.ts: existing poll test in therefreshWorkspacespath now asserts the exact argument list.Why
#1639 renamed
devsy workspace list --skip-proto--skip-platform. The CLI keeps--skip-proas a hidden alias, but the desktop app still sent the old flag.Tier
T2: desktop passes the new flag; old CLI alias still works; CLI released together with desktop bundle.
Validation
npm ci --ignore-scriptsindesktop/npx vitest run src/main/__tests__/watcher.test.ts src/main/__tests__/cli.test.ts: 58 passednpm run test(desktop): 87 files, 1098 tests passednpm run check(svelte-check): 0 errorsnpm run lint:check: exit 0, 3 pre-existing warnings, none in touched filesnpx biome checkon the two touched files: cleannpx tsc --noEmit -p tsconfig.json: errors only in untouchedsrc/main/tray.tsgit grep -n 'skip-pro' -- desktop: onlydesktop/e2e/fixtures/mock-devsy.cjsmise run cli:formatnot run: no Go files changedCodeRabbit CLI
coderabbit review --agent --committed --base-commit 66e21d8460f2261f36b5714ea123051f7fa6b86d: completed, 0 findings, 2 files reviewed.Auto-merge
Eligible: yes. Non-test changed lines: 6 (5 added, 1 removed) in 1 file. Behaviour-preserving for the CLI surface; no tests removed or weakened.
Remaining
Desktop
--skip-prohits left: onlydesktop/e2e/fixtures/mock-devsy.cjs, which accepts both flags and is intentionally untouched.Summary by CodeRabbit