-
-
Notifications
You must be signed in to change notification settings - Fork 19
refactor: migrate from submodules to multi-root workspace for plugins #198
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
Conversation
- Updated LEDMatrix.code-workspace to include all plugin repos as root folders - Removed symlinks from plugin-repos/ and plugins/ directories - Updated .gitignore to reflect new plugin management approach - Added setup_plugin_repos.py script for managing plugin symlinks (if needed) - Added MULTI_ROOT_WORKSPACE_SETUP.md documentation Plugins are now managed as independent repositories via multi-root workspace, allowing for easier development and independent updates without modifying the LEDMatrix project structure.
📝 WalkthroughWalkthroughThis PR replaces per-plugin git submodule pointers with a multi-root VS Code workspace and adds tooling/docs to manage plugins as separate repositories via symlinks; it also removes per-plugin .gitignore whitelist entries and adds a script to create symlinks into Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as "Developer"
participant Workspace as "LEDMatrix.code-workspace"
participant Script as "scripts/setup_plugin_repos.py"
participant GitHub as "GitHub dir (../ledmatrix-*/)"
participant FS as "Local FS (plugin-repos/)"
Dev->>Workspace: open multi-root workspace
Dev->>Script: run setup_plugin_repos.py
Script->>Workspace: read folder entries (../...)
Script->>GitHub: check existence of target repo paths
alt repo exists
Script->>FS: create relative symlink in plugin-repos/
FS-->>Script: success
else missing repo
Script-->>Dev: log error (missing source)
end
Script-->>Dev: report created/skipped/errors and exit code
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@docs/MULTI_ROOT_WORKSPACE_SETUP.md`:
- Around line 54-59: The docs show a deprecated command "python3
scripts/clone_plugin_repos.py" as an actionable step; remove that line or
replace it with the current recommended action so readers aren't confused. Edit
MULTI_ROOT_WORKSPACE_SETUP.md to delete the "python3
scripts/clone_plugin_repos.py # Note: This script was removed, see below" entry
(or update it to reference the new script or explicit manual steps), and ensure
any surrounding text clarifies that the old script clone_plugin_repos.py is
removed and points to the correct alternative.
- Around line 16-29: The code fence containing the directory tree in
MULTI_ROOT_WORKSPACE_SETUP.md is missing a language tag (triggering MD040);
update the opening triple-backtick for the directory-tree block to include a
language identifier such as "text" (i.e., change ``` to ```text) so the block is
recognized as plain text—look for the directory-tree code fence showing
"/home/chuck/Github/" and modify its opening backticks accordingly.
In `@scripts/setup_plugin_repos.py`:
- Around line 22-30: In get_workspace_plugins(), wrap the json.load call in a
try/except that catches json.JSONDecodeError specifically (in addition to
existing file existence check) and handle it by printing or logging a clear,
user-friendly message and returning an empty list; ensure the exception handling
references the workspace variable/file and leaves other exceptions unmodified so
only parse errors are swallowed.
🧹 Nitpick comments (4)
docs/MULTI_ROOT_WORKSPACE_SETUP.md (2)
16-35: Use a placeholder path instead of a hard-coded/home/chuck/Github.
Consider$HOME/GitHubor<parent_dir>so the doc applies beyond a single machine.
112-117: Document the plugin pre-push hook setup.
Add a step to copyscripts/git-hooks/pre-push-plugin-versioninto each plugin repo’s.git/hooks/pre-pushso version bumps stay consistent.Based on learnings, consider adding this to the “Adding New Plugins” steps.
scripts/setup_plugin_repos.py (2)
75-106: Avoid broadExceptionfor filesystem operations.
Narrow toOSError(and optionallyRuntimeError) so unexpected issues aren’t silently swallowed.As per coding guidelines, catch specific exceptions rather than broad `Exception`.♻️ Suggested adjustment
- except Exception as e: + except OSError as e: print(f" ⚠️ {plugin['name']} - error checking link: {e}") - link_path.unlink() + try: + link_path.unlink() + except OSError as unlink_err: + print(f" ⚠️ {plugin['name']} - failed to remove link: {unlink_err}") + errors += 1 + continue ... - except Exception as e: + except OSError as e: print(f" ✗ {plugin['name']} - failed to create link: {e}") errors += 1
46-64: Consider structured logging and type hints for maintainability.
Replacingloggingmodule (with a[Plugin Setup]context) and adding return type hints would improve Pi troubleshooting and clarity.As per coding guidelines, use structured logging with context and add type hints.
- Remove deprecated clone_plugin_repos.py command reference - Add language tag to directory tree code fence (fixes MD040) - Add JSONDecodeError handling in setup_plugin_repos.py with user-friendly error messages
Plugins are now managed as independent repositories via multi-root workspace, allowing for easier development and independent updates without modifying the LEDMatrix project structure.
Summary by CodeRabbit
Documentation
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.