opt.: include core repos via submodules#1046
Conversation
Summary of ChangesHello @lollipopkit, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refactors how several core dependencies are managed within the project. Instead of relying on direct Git references for these packages, they are now included as Git submodules. This change aims to provide a more integrated and potentially more stable development environment by allowing local modifications and consistent versioning of these internal dependencies. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
📝 WalkthroughWalkthroughThis pull request migrates external package dependencies to git submodules. The changes introduce five git submodules (dartssh2, xterm, fl_lib, fl_build, and server_box_monitor) by creating a new .gitmodules configuration file, updating pubspec.yaml to reference these packages via local paths instead of git URLs, adding submodule commit references, and updating the CI workflow to fetch submodules recursively with the analysis command switched from Dart to Flutter analyzer. Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
✅ Files skipped from review due to trivial changes (5)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (1)pubspec.yaml📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (7)📓 Common learnings📚 Learning: 2025-10-08T09:36:41.682ZApplied to files:
📚 Learning: 2025-10-08T09:36:41.682ZApplied to files:
📚 Learning: 2025-10-08T09:36:41.682ZApplied to files:
📚 Learning: 2026-01-17T04:24:16.501ZApplied to files:
📚 Learning: 2025-10-08T09:36:41.682ZApplied to files:
📚 Learning: 2025-10-08T09:36:41.682ZApplied to files:
🔇 Additional comments (1)
✏️ Tip: You can disable this entire section by setting Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request switches several dependencies from being consumed as pub git references to being included as Git submodules. This is done to include the repositories directly, as requested in an issue. The changes involve adding a .gitmodules file to define the submodules, updating pubspec.yaml to use local path dependencies, and refreshing pubspec.lock accordingly. My review focuses on the configuration of the new submodules. I have one suggestion regarding the submodule configuration to improve build reproducibility.
| [submodule "dartssh2"] | ||
| path = dartssh2 | ||
| url = https://github.com/lollipopkit/dartssh2 | ||
| branch = master | ||
| [submodule "xterm"] | ||
| path = xterm | ||
| url = https://github.com/lollipopkit/xterm.dart | ||
| branch = master | ||
| [submodule "fl_lib"] | ||
| path = fl_lib | ||
| url = https://github.com/lollipopkit/fl_lib | ||
| branch = main | ||
| [submodule "fl_build"] | ||
| path = fl_build | ||
| url = https://github.com/lppcg/fl_build.git | ||
| branch = main | ||
| [submodule "server_box_monitor"] | ||
| path = server_box_monitor | ||
| url = https://github.com/lollipopkit/server_box_monitor | ||
| branch = main |
There was a problem hiding this comment.
Using the branch property for submodules can lead to non-reproducible builds. When a developer runs git submodule update --remote, they will pull the latest commit from the tracked branch, which may differ from the commit other developers or the CI system are using. This can cause inconsistencies and "it works on my machine" issues.
For better stability and reproducibility, it's recommended to pin submodules to specific commits by omitting the branch property. Updates then become a deliberate action: a developer manually checks out a specific commit or tag within the submodule and then commits the new submodule hash to the parent repository. This ensures everyone is using the exact same version of the dependency.
Consider removing the branch properties to enforce this safer workflow.
There was a problem hiding this comment.
Thanks for the suggestion. Keeping branch in .gitmodules is intentional for this issue (#1007), which explicitly requests keeping these direct-included repos on latest upstream origin/main/origin/master. We still keep reproducibility via committed submodule SHA in the parent repo; branch here is only to define the target branch for explicit git submodule update --remote updates.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
Devin Review found 1 potential issue.
🐛 1 issue in files not directly in the diff
🐛 Analysis CI workflow missing submodule checkout will fail on flutter pub get (.github/workflows/analysis.yml:19-21)
The analysis workflow will fail because it does not checkout submodules, but the project now depends on local path dependencies (dartssh2, xterm, fl_lib, fl_build) that require the submodules to be initialized.
Root Cause
The pubspec.yaml now uses path dependencies:
dartssh2:
path: dartssh2
xterm:
path: xterm
fl_lib:
path: fl_lib
fl_build:
path: fl_buildHowever, .github/workflows/analysis.yml:19-21 only has:
- uses: actions/checkout@v6
with:
fetch-depth: 1Without submodules: recursive, the submodule directories will be empty, causing flutter pub get at line 28 to fail with errors like "Could not find package dartssh2 at path dartssh2".
In contrast, .github/workflows/release.yml correctly specifies submodules: recursive in all its checkout steps (lines 23-24, 72-73, 111-112, 141-142, 172-173).
Impact: All PR checks and pushes to main will fail the analysis workflow, breaking CI.
View 3 additional findings in Devin Review.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
1 participant
Summary
dartssh2,xterm,fl_lib,fl_build, andserver_box_monitordartssh2,xterm,fl_lib, and dev dependencyfl_build.gitmodules(main/master)Why
Validation
flutter testflutter analyze lib testCloses #1007
Summary by CodeRabbit