build(sdk): add sdk as git submodule and update deps to path overrides#3110
build(sdk): add sdk as git submodule and update deps to path overrides#3110
Conversation
…only by backup/restore before inline updates
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughAdds the SDK as a Git submodule and enables recursive submodule checkout across CI workflows. Updates one workflow to update the SDK submodule in-place instead of cloning. Switches several Dart dependencies to local path-based resolutions via the sdk/ submodule, adds dependency_overrides, and updates documentation for submodule usage. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer/Trigger
participant GH as GitHub Actions Runner
participant Repo as Repo (with submodule)
participant SDK as SDK Remote (komodo-defi-sdk-flutter)
Dev->>GH: Run sdk-integration-preview
GH->>Repo: actions/checkout@v4 (submodules: recursive)
note right of GH: Submodules initialized recursively
GH->>Repo: cd sdk && git fetch origin
GH->>Repo: git checkout <sdk_branch>
GH->>Repo: git pull origin <sdk_branch>
Repo-->>SDK: Fetch/Update from remote
GH->>GH: Continue remaining jobs (build/test)
note over GH: Uses local path-based deps via dependency_overrides
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR introduces the komodo-defi-sdk-flutter repository as a git submodule and migrates SDK package dependencies from pub.dev hosted versions to local path overrides. This change enables local development, testing, and hotfix workflows for SDK packages while maintaining consistency across development environments.
- Adds SDK as git submodule in
sdk/directory tracking thedevbranch - Updates all SDK package dependencies to use local path references instead of hosted versions
- Adds comprehensive documentation for submodule management workflows
Reviewed Changes
Copilot reviewed 17 out of 20 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| sdk | New git submodule pointing to komodo-defi-sdk-flutter repository |
| pubspec.yaml | Updates SDK dependencies to path overrides and adds dependency_overrides section |
| packages/komodo_ui_kit/pubspec.yaml | Updates nested package dependencies to use path overrides |
| docs/SDK_SUBMODULE_MANAGEMENT.md | New comprehensive guide for working with the SDK submodule |
| docs/PROJECT_SETUP.md | Updates setup instructions to include submodule initialization |
| docs/CLONE_REPOSITORY.md | Enhanced cloning instructions with submodule support |
| .gitmodules | Git submodule configuration file |
| Multiple workflow files | Updates GitHub Actions to initialize submodules during checkout |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/docker-android-build.yml (1)
3-22: Trigger on submodule pointer changes; includesdk(gitlink) in path filters.When only the SDK submodule pointer changes in the superproject, GitHub reports the changed path as sdk (a gitlink), not sdk/**. Your current paths filter won’t fire for that, so the Docker image won’t rebuild on SDK bumps unless other files also change.
Add both sdk and sdk/** to be safe:
on: pull_request: branches: [ dev, main, release/*, hotfix/*, bugfix/*, feature/*, chore/*, build/*, ci/*, ] paths: - ".docker/**" - "android/**" - "pubspec.yaml" - "pubspec.lock" + - "sdk" + - "sdk/**" workflow_dispatch:
🧹 Nitpick comments (33)
.github/workflows/mobile-builds.yml (1)
34-35: Optionally pass an explicit token and consider fetch-depth if submodule history is needed.
- If the SDK submodule ever becomes private, explicitly pass token to Checkout to avoid surprises.
- If any downstream step relies on submodule history (e.g., git describe), set fetch-depth: 0.
Suggested tweak:
- name: Checkout code uses: actions/checkout@v4 with: submodules: recursive + token: ${{ secrets.GITHUB_TOKEN }} + fetch-depth: 0.gitmodules (1)
1-4: Optional: document SSH remotes for contributors who push to the SDK.Keeping HTTPS in .gitmodules is fine for CI and new contributors. For devs who need to push inside sdk/, consider documenting a per-user local override:
git -C sdk remote set-url origin git@github.com:KomodoPlatform/komodo-defi-sdk-flutter.git.github/workflows/docker-android-build.yml (1)
28-32: Optional: align Checkout token and fetch-depth with mobile workflow.If you adopt token and fetch-depth adjustments in other workflows, mirror them here for consistency.
- name: Checkout uses: actions/checkout@v4 with: submodules: recursive + token: ${{ secrets.GITHUB_TOKEN }} + fetch-depth: 0sdk (3)
1-1: Harden CI for submodules to avoid flaky checkouts and speed things up.Ensure every workflow that runs Dart/Flutter tasks does a consistent checkout of submodules and that dependency resolution sees the local path packages.
Suggested steps (outside this file):
- In actions/checkout, use fetch-depth: 0 and submodules: recursive.
- Immediately follow with
git submodule sync --recursiveandgit submodule update --init --recursive --depth 1.- Cache pub to offset the lack of pub.dev tarballs when using path overrides.
Example snippet:
- uses: actions/checkout@v4 + uses: actions/checkout@v4 with: - submodules: recursive + submodules: recursive + fetch-depth: 0 +shell: bash +run: | + git submodule sync --recursive + git submodule update --init --recursive --depth 1 + dart --version || true + flutter --version || true + dart pub cache repair || true
1-1: Guard rails for path dependency overrides.Switching production builds to dependency_overrides pointed at sdk/packages is fine for this PR’s objective, but it carries risks:
- Overrides apply repo-wide and can mask version conflicts.
- Publishing to pub.dev while overrides are present is hazardous.
Add safety in the root pubspec.yaml (outside this file):
+publish_to: "none"And consider using a
pubspec_overrides.yamlfor local-only overrides when feasible. If CI must use overrides, keep them documented in docs/SDK_SUBMODULE_MANAGEMENT.md and ensure release pipelines validate without overrides before shipping.
1-1: Document developer workflow for the new submodule.Reduce onboarding friction by adding a short, copy-pasteable block to the setup docs that matches CI:
Proposed doc snippet (outside this file):
# One-time: git submodule update --init --recursive # Keep in sync: git submodule sync --recursive git submodule update --init --recursive # Update SDK to latest dev (intentional only): # git -C sdk fetch origin dev && git -C sdk checkout <commit-or-tag> && git add sdk && git commit -m "bump sdk".github/workflows/desktop-builds.yml (1)
38-39: Good call enabling recursive submodules; add fetch-depth/token for reliability with private submodules.actions/checkout@v4 will init/fetch submodules, but:
- If sdk is private or moves private in future, GITHUB_TOKEN on the top-level checkout won’t authenticate across repos. Provide a PAT with repo scope via token to avoid intermittent failures.
- Adding fetch-depth: 0 helps prevent shallow submodule fetch issues when submodule pointers reference non-tip commits.
Proposed tweak:
- uses: actions/checkout@v4 with: + fetch-depth: 0 + # If sdk submodule ever becomes private, switch to a PAT with repo scope: + # token: ${{ secrets.SUBMODULES_TOKEN }} submodules: recursivePlease confirm whether komodo-defi-sdk-flutter is public for all workflows; if not, we should standardize on a SUBMODULES_TOKEN secret.
.github/workflows/roll-sdk-packages.yml (1)
39-44: Submodules: recursive added correctly; verify auth scope when the SDK repo is private.This job already uses fetch-depth: 0 (nice). If the sdk submodule is private, GITHUB_TOKEN will not grant cross-repo access; use a PAT secret with repo scope.
Suggested adjustment:
- name: Checkout code uses: actions/checkout@v4 with: fetch-depth: 0 - token: ${{ secrets.GITHUB_TOKEN }} + # Prefer PAT with repo scope if submodule is private: + # token: ${{ secrets.SUBMODULES_TOKEN }} submodules: recursiveIf the submodule remains public, current config is fine; keep as-is.
.github/workflows/firebase-hosting-merge.yml (1)
27-28: Enable deep history and consider PAT for submodule auth (defensive hardening).Mirroring the other workflows, consider fetch-depth: 0 and (optionally) a PAT for private submodules.
- name: Setup GH Actions uses: actions/checkout@v4 with: + fetch-depth: 0 + # token: ${{ secrets.SUBMODULES_TOKEN }} # only if sdk is private submodules: recursive.github/workflows/unit-tests-on-pr.yml (1)
17-18: Nice; add fetch-depth: 0 to avoid shallow submodule pitfalls during tests.Unit tests that import path-based packages from the sdk submodule will be sensitive to submodule fetch state. fetch-depth: 0 is a low-cost guard.
- name: Setup GH Actions uses: actions/checkout@v4 with: + fetch-depth: 0 submodules: recursive.github/workflows/ui-tests-on-pr.yml (1)
45-46: Consistent submodule init; consider centralizing this pattern via a reusable workflow.You now repeat the same checkout options across many workflows. A reusable workflow (e.g., .github/workflows/_common-checkout.yml) would DRY this and let you flip tokens/flags in one place.
Example usage:
# .github/workflows/_common-checkout.yml on: workflow_call: inputs: use_pat: type: boolean default: false jobs: checkout: runs-on: ubuntu-latest outputs: done: ok steps: - uses: actions/checkout@v4 with: fetch-depth: 0 submodules: recursive token: ${{ inputs.use_pat && secrets.SUBMODULES_TOKEN || secrets.GITHUB_TOKEN }}Then call it at the top of each workflow before other steps.
.github/workflows/validate-code-guidelines.yml (1)
18-19: Good addition; consider fetch-depth for submodules reliabilityAdding submodules: recursive is correct. To avoid shallow clone edge-cases with submodules (e.g., checking out non-default branches/tags inside submodules during later steps), consider setting fetch-depth: 0. This is optional but improves robustness.
- name: Setup GH Actions uses: actions/checkout@v4 with: submodules: recursive + fetch-depth: 0.github/workflows/firebase-hosting-pull-request.yml (1)
36-37: Enable full history to reduce submodule edge-casesSame note as in other workflows: fetch-depth: 0 can prevent issues when submodules need non-shallow history. Low risk, minor performance impact.
- name: Setup GH Actions uses: actions/checkout@v4 with: submodules: recursive + fetch-depth: 0packages/komodo_ui_kit/pubspec.yaml (2)
16-16: Update comments to reflect submodule usage (not a symlink)The comments still mention a required symlink. The repo now uses an sdk submodule at sdk/. Suggest aligning the wording.
- path: ../../sdk/packages/komodo_defi_types # Option 1: Local directory. Requires symlink to the SDK in the root of the project + path: ../../sdk/packages/komodo_defi_types # Option 1: Local directory via the sdk/ submodule tracked in this repo- path: ../../sdk/packages/komodo_ui # Option 1: Local directory. Requires symlink to the SDK in the root of the project + path: ../../sdk/packages/komodo_ui # Option 1: Local directory via the sdk/ submodule tracked in this repoAlso applies to: 24-24
34-40: Confirm duplication between direct path deps and dependency_overridesYou’re declaring both direct path dependencies and dependency_overrides for the same packages. That’s valid and ensures transitive references resolve locally, but it’s easy to drift across packages. If the root pubspec.yaml already enforces these overrides across the workspace, consider dropping per-package overrides to reduce duplication.
Would you like a quick yq-based script to audit and normalize overrides across all package pubspec.yaml files?
.github/workflows/sdk-integration-preview.yml (4)
44-45: Add fetch-depth and safe.directory for submodule operationsWhen manipulating the submodule, Git may complain about ownership in CI; also shallow history can occasionally bite when switching branches/tags.
- name: Setup GH Actions uses: actions/checkout@v4 with: submodules: recursive + fetch-depth: 0 + + - name: Configure Git safe directory for submodule + shell: bash + run: | + git config --global --add safe.directory "$GITHUB_WORKSPACE" + git config --global --add safe.directory "$GITHUB_WORKSPACE/sdk"
47-56: Make SDK branch switching robust (branch/tag support, non-existent local branch)Current sequence works for a normal branch but fails for tags or if the branch doesn’t exist locally with correct tracking. Also, explicitly re-fetching and using switch/checkout patterns improves reliability.
- - name: Update SDK to specified branch + - name: Update SDK to specified branch or tag shell: bash run: | - echo "Updating SDK submodule to branch ${{ inputs.sdk_branch }}..." - cd sdk - git fetch origin - git checkout ${{ inputs.sdk_branch }} - git pull origin ${{ inputs.sdk_branch }} - cd .. - echo "SDK updated to branch ${{ inputs.sdk_branch }}" + set -euo pipefail + BR="${{ inputs.sdk_branch }}" + echo "Updating SDK submodule to '$BR'..." + cd sdk + git remote -v + git fetch --all --tags --prune origin + if git ls-remote --exit-code --heads origin "$BR" >/dev/null 2>&1; then + # Create or reset local branch to track origin/BR + git switch -C "$BR" --track "origin/$BR" || git switch "$BR" + git pull --ff-only origin "$BR" + echo "Checked out branch '$BR' and fast-forwarded." + elif git rev-parse -q --verify "refs/tags/$BR" >/dev/null || git ls-remote --exit-code --tags origin "$BR" >/dev/null 2>&1; then + git checkout "tags/$BR" + echo "Checked out tag '$BR'." + else + echo "Error: '$BR' is neither a branch nor a tag in origin." >&2 + exit 1 + fi + cd .. + echo "SDK updated to '$BR'" ls -la sdk/
89-119: Avoid brittle sed on YAML; prefer yq or a script guarded by patternsThe sed ranges depend on comments like ref: dev and exact ordering. Minor formatting changes will break these edits. Consider switching to yq for structural YAML updates.
Example with yq v4 (Ubuntu has snap or pip installable):
- - name: Update pubspec.yaml to use local SDK paths + - name: Update pubspec.yaml to use local SDK paths (yq) shell: bash run: | - echo "Updating pubspec.yaml files to use local SDK paths..." - # ... multiple sed commands ... - echo "Pubspec.yaml files updated successfully" + set -euo pipefail + echo "Updating pubspec.yaml files to use local SDK paths..." + pipx install yq || pip install yq + yq -i ' + .dependencies.komodo_cex_market_data = {"path":"sdk/packages/komodo_cex_market_data"} | + .dependencies.komodo_defi_sdk = {"path":"sdk/packages/komodo_defi_sdk"} | + .dependencies.komodo_defi_types = {"path":"sdk/packages/komodo_defi_types"} | + .dependencies.komodo_ui = {"path":"sdk/packages/komodo_ui"} + ' pubspec.yaml + yq -i ' + .dependencies.komodo_defi_types = {"path":"../../sdk/packages/komodo_defi_types"} | + .dependencies.komodo_ui = {"path":"../../sdk/packages/komodo_ui"} + ' packages/komodo_ui_kit/pubspec.yaml + echo "pubspec.yaml files updated successfully"If you prefer keeping sed, at least guard for missing markers and fail fast.
165-165: Fix garbled character in log outputThere’s a replacement character (�) before “Channel ID”. Replace it with a valid emoji or plain text.
- echo "� Channel ID: ${{ steps.get_preview_channel_id.outputs.preview_channel_id }}" + echo "🛰️ Channel ID: ${{ steps.get_preview_channel_id.outputs.preview_channel_id }}"docs/SDK_SUBMODULE_MANAGEMENT.md (6)
22-23: Clarify what submodule checkout actually does“Checkout the tracked commit on the dev branch” can be misleading. Submodules check out the exact commit recorded by the superproject; tracking a branch requires submodule..branch config and an update action.
-This will clone the SDK repository into the `sdk/` folder and checkout the tracked commit on the `dev` branch. +This clones the SDK repository into `sdk/` and checks out the exact commit recorded by the wallet repo (currently following `dev`). Branch tracking requires explicit configuration and update.
111-121: Re-evaluate “Keep the submodule on dev for production builds”Using dev for production builds is risky unless your release process treats dev as a stable train. Consider pinning to a release tag or a known-good commit for production.
Proposed wording:
-- **Keep the submodule on `dev` branch** for production builds +- **Pin the submodule to a release tag or vetted commit** for production builds; use `dev` during active development and testing
124-127: Grammar and clarity: “check out” vs “checkout”Use “check out” (verb) and tighten the sentence.
-**Don't work in detached HEAD state** - always checkout a branch in the submodule +**Don't work in a detached HEAD state** — always check out a branch in the submodule
91-103: Branch switching: prefer git switch and add a note about tagsTo avoid detached HEADs and improve UX, recommend git switch and mention tags.
-git fetch origin -git checkout feature/some-feature-branch +git fetch --all --tags origin +git switch -C feature/some-feature-branch --track origin/feature/some-feature-branch +# If testing a tag: +# git checkout tags/vX.Y.Z
185-189: Grammar tweakMinor improvement.
-4. Consider rolling back to previous working commit temporarily +4. Consider rolling back to a previous working commit temporarily
204-207: Verify referenced doc exists or update the linkSDK_DEPENDENCY_MANAGEMENT.md may not exist in this repo. If it’s not present, either add it or replace the link with the section in this document or an existing doc.
-- [SDK_DEPENDENCY_MANAGEMENT.md](SDK_DEPENDENCY_MANAGEMENT.md) - General dependency management guidelines +- [PROJECT_SETUP.md](PROJECT_SETUP.md) - General dependency management guidelines (update if a dedicated doc is added)docs/PROJECT_SETUP.md (2)
33-39: Clarify when submodule init is required; add a convenience tipGood addition. Recommend telling users they can skip this if they cloned with --recurse-submodules, and add the helpful submodule.recurse config.
Apply this diff to extend the step:
5. **Initialize SDK submodule**: After cloning, initialize the komodo-defi-sdk-flutter submodule: ```bash cd komodo-wallet git submodule update --init --recursive ``` + + Note: If you cloned with `--recurse-submodules`, you can skip this step. + Optional (recommended): make submodule-aware Git commands default to recurse: + ```bash + git config submodule.recurse true + ```
17-17: Typos: “Command Palette”, “Xcode”Minor polish:
- Line 17: “Command Pallette” → “Command Palette”
- Line 25: “xCode” → “Xcode”
- - enable `Dart: Use recommended settings` via the Command Pallette + - enable `Dart: Use recommended settings` via the Command Palette - - [xCode](https://developer.apple.com/xcode/) | 16.2 (macOS only) + - [Xcode](https://developer.apple.com/xcode/) | 16.2 (macOS only)Also applies to: 25-25
docs/CLONE_REPOSITORY.md (3)
21-29: Add a convenience tip after SSH cloneAfter the SSH one-liner, suggest setting submodule.recurse so future Git commands recurse automatically.
```bash git clone --recurse-submodules git@github.com:KomodoPlatform/komodo-wallet.git
+Optional (recommended):
+bash +cd komodo-wallet +git config submodule.recurse true +--- `47-56`: **Strengthen submodule verification with ‘git submodule status’** Listing the directory works; adding a git-native check makes it unambiguous that sdk is a submodule and points to the expected commit. ```diff ## Verifying Submodule Setup After cloning, verify that the SDK submodule was initialized correctly: ```bash -ls -la sdk/ +ls -la sdk/ +# Additionally, verify via Git: +git submodule status sdkYou should see the komodo-defi-sdk-flutter repository contents in the
sdk/directory.
+Andgit submodule status sdkshould print a line with a commit SHA (e.g., " d0fb98… sdk").--- `5-19`: **Optional: Present the one-command HTTPS clone first** Since it’s the simplest happy path, consider placing the --recurse-submodules one-liner before the two-step clone + init flow. </blockquote></details> <details> <summary>pubspec.yaml (3)</summary><blockquote> `46-53`: **Outdated comment text: replace “Requires symlink…” with “Requires SDK submodule at sdk/”** These path deps now resolve via the sdk submodule, not a symlink. Update the comment to avoid confusion for new contributors. ```diff komodo_cex_market_data: - path: sdk/packages/komodo_cex_market_data # Option 1: Local directory. Requires symlink to the SDK in the root of the project + path: sdk/packages/komodo_cex_market_data # Option 1: Local directory. Requires the SDK submodule at sdk/ @@ dragon_logs: - path: sdk/packages/dragon_logs # Option 1: Local directory. Requires symlink to the SDK in the root of the project + path: sdk/packages/dragon_logs # Option 1: Local directory. Requires the SDK submodule at sdk/ @@ dragon_charts_flutter: - path: sdk/packages/dragon_charts_flutter # Option 1: Local directory. Requires symlink to the SDK in the root of the project + path: sdk/packages/dragon_charts_flutter # Option 1: Local directory. Requires the SDK submodule at sdk/ @@ komodo_defi_sdk: - path: sdk/packages/komodo_defi_sdk # Option 1: Local directory. Requires symlink to the SDK in the root of the project + path: sdk/packages/komodo_defi_sdk # Option 1: Local directory. Requires the SDK submodule at sdk/ @@ komodo_defi_types: - path: sdk/packages/komodo_defi_types # Option 1: Local directory. Requires symlink to the SDK in the root of the project + path: sdk/packages/komodo_defi_types # Option 1: Local directory. Requires the SDK submodule at sdk/ @@ komodo_ui: - path: sdk/packages/komodo_ui # Option 1: Local directory. Requires symlink to the SDK in the root of the project + path: sdk/packages/komodo_ui # Option 1: Local directory. Requires the SDK submodule at sdk/Also applies to: 55-61, 136-142, 156-162, 164-170, 172-178
198-200: Trailing whitespace and minor grammar clean-upRemove trailing spaces at lines 198 and 200. Also, the comment reads well now (“are” present). Keep it as-is after trimming.
dependency_overrides: # Temporary until Flutter's pinned version is updated intl: ^0.20.2 - + # Force all komodo_ packages to use local path versions - # Internal SDK interdependencies are hosted versions + # Internal SDK interdependencies are hosted versions
62-73: Versioning strategy consistency (optional)There’s a mix of pinned (e.g., http: 1.4.0) and ranged (e.g., ^, <=) constraints. If intentional, consider documenting the policy briefly (e.g., “pin where security/ABI-sensitive, caret otherwise”) to reduce churn.
Also applies to: 85-101, 111-121, 123-131, 143-154
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
packages/komodo_ui_kit/pubspec.lockis excluded by!**/*.lockpubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (18)
.github/workflows/desktop-builds.yml(1 hunks).github/workflows/docker-android-build.yml(1 hunks).github/workflows/firebase-hosting-merge.yml(1 hunks).github/workflows/firebase-hosting-pull-request.yml(1 hunks).github/workflows/mobile-builds.yml(1 hunks).github/workflows/roll-sdk-packages.yml(1 hunks).github/workflows/sdk-integration-preview.yml(1 hunks).github/workflows/ui-tests-on-pr.yml(1 hunks).github/workflows/unit-tests-on-pr.yml(1 hunks).github/workflows/validate-code-guidelines.yml(1 hunks).gitignore(0 hunks).gitmodules(1 hunks)docs/CLONE_REPOSITORY.md(1 hunks)docs/PROJECT_SETUP.md(1 hunks)docs/SDK_SUBMODULE_MANAGEMENT.md(1 hunks)packages/komodo_ui_kit/pubspec.yaml(1 hunks)pubspec.yaml(4 hunks)sdk(1 hunks)
💤 Files with no reviewable changes (1)
- .gitignore
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-04-01T15:51:37.060Z
Learnt from: takenagain
PR: KomodoPlatform/komodo-wallet#2566
File: lib/bloc/coins_bloc/coins_bloc.dart:10-10
Timestamp: 2025-04-01T15:51:37.060Z
Learning: In the Komodo Wallet project, part files share imports with their parent files. The import for `app_config.dart` in `coins_bloc.dart` is necessary because the part file `coins_state.dart` uses `excludedAssetList` from that package.
Applied to files:
packages/komodo_ui_kit/pubspec.yaml
🪛 LanguageTool
docs/SDK_SUBMODULE_MANAGEMENT.md
[style] ~42-~42: Consider shortening or rephrasing this to strengthen your wording.
Context: ...ges (Hotfix Workflow) When you need to make changes to the SDK: 1. **Create a hotfix branch i...
(MAKE_CHANGES)
[grammar] ~124-~124: There might be a mistake here.
Context: ...g the SDK ### Don'ts ❌ - Don't work in detached HEAD state - always checkout...
(QB_NEW_EN)
[grammar] ~188-~188: There might be a mistake here.
Context: .../release notes 4. Consider rolling back to previous working commit temporarily ##...
(QB_NEW_EN)
docs/CLONE_REPOSITORY.md
[grammar] ~23-~23: There might be a mistake here.
Context: ...connecting-to-github-with-ssh) properly. Then you should be able to run: ```bash...
(QB_NEW_EN)
docs/PROJECT_SETUP.md
[grammar] ~40-~40: There might be a mistake here.
Context: ...nd run the App for each target platform: - Web - [Androi...
(QB_NEW_EN)
[grammar] ~41-~41: There might be a mistake here.
Context: ...App for each target platform: - Web - [Android mobile](BUILD_RUN_APP.md#android...
(QB_NEW_EN)
[grammar] ~42-~42: There might be a mistake here.
Context: ...LD_RUN_APP.md#web) - Android mobile - iOS mobile (macOS...
(QB_NEW_EN)
[grammar] ~43-~43: There might be a mistake here.
Context: ...LD_RUN_APP.md#android) - iOS mobile (macOS host only) - [macOS desktop]...
(QB_NEW_EN)
[grammar] ~43-~43: There might be a mistake here.
Context: ...(BUILD_RUN_APP.md#ios) (macOS host only) - [macOS desktop](BUILD_RUN_APP.md#macos-de...
(QB_NEW_EN)
[grammar] ~44-~44: There might be a mistake here.
Context: ..._APP.md#macos-desktop) (macOS host only) - [Windows desktop](BUILD_RUN_APP.md#window...
(QB_NEW_EN)
[grammar] ~45-~45: There might be a mistake here.
Context: ....md#windows-desktop) (Windows host only) - [Linux desktop](BUILD_RUN_APP.md#linux-de...
(QB_NEW_EN)
[grammar] ~46-~46: There might be a mistake here.
Context: ..._APP.md#linux-desktop) (Linux host only) 7. Build release version...
(QB_NEW_EN)
🪛 GitHub Actions: takenagain is running unit tests on PR 🚀
sdk
[error] 39-39: No named parameter with the name 'fiatCurrency'.
🪛 GitHub Actions: takenagain is running UI tests on PR 🚀
sdk
[error] 39-39: No named parameter with the name 'fiatCurrency'.
🪛 YAMLlint (1.37.1)
pubspec.yaml
[error] 198-198: trailing spaces
(trailing-spaces)
[error] 200-200: trailing spaces
(trailing-spaces)
🔇 Additional comments (12)
.github/workflows/mobile-builds.yml (2)
34-35: Good move enabling recursive submodules in Checkout.This is necessary now that pubspecs point to local paths under sdk/. Nothing else required for a basic public submodule.
72-79: Given the results of the GitHub issue:
- If the issue state is closed, then the regression has been fixed upstream and you can safely remove the workaround.
- If the issue state is open, you should guard the step by Flutter version (as originally suggested) or keep the workaround.
Please share the script output (
state,closed_at, andmilestone.title), so I can confirm which action to take..gitmodules (2)
1-4: Submodule definition looks correct and matches the repo structure.
- Path is sdk, URL is HTTPS (good for CI), branch is dev (for local update --remote workflows).
1-4: Confirm CI does not use--remotewhen updating submodules
Our search found only safegit submodule update --init --recursiveinvocations (in documentation), with no occurrences of--remotein CI workflows or scripts. To maintain reproducible builds, please double-check that no CI job or external script invokes:
git submodule update --remote- Any wrapper that implicitly passes
--remoteThis ensures the SDK submodule (pinned to the dev branch in .gitmodules) won’t unexpectedly drift to its tip.
.github/workflows/docker-android-build.yml (1)
30-31: Recursive submodule checkout is appropriate here as well.Required for Docker builds that depend on local path packages inside sdk/.
sdk (2)
1-1: Submodule bump itself looks fine.Pointer updates are minimal and scoped. Once the 'fiatCurrency' call sites are fixed, builds should stabilize.
1-1: Initialize SDK submodule and verify parameter renames
It looks like thesdksubmodule isn’t populated in this environment, so we can’t inspect its updated API signatures automatically. Please:
- Run:
git submodule update --init sdk- Re-run the search script to locate the new parameter names in the SDK for:
getFiatPrice(...)getPortfolioGrowthChart(...)RampQuoteResultForPaymentMethod(...)constructorProfitLossCache.getPrimaryKey(...)- Update all call sites (in app code and tests) by replacing
fiatCurrency:with the correct new named parameter(s) as defined in the SDK.Once you’ve confirmed the replacement names, share one representative usage if you’d like a focused diff.
.github/workflows/validate-code-guidelines.yml (1)
21-34: Sanity check: analyzers pick up submodule packagesNow that dependencies resolve to sdk/ paths, ensure flutter analyze runs after submodules are initialized and pub get has resolved local paths. Your sequence already installs deps and generates assets before analyze, so this should be fine. Just a heads-up to monitor analyzer failures tied to submodule resolution.
docs/SDK_SUBMODULE_MANAGEMENT.md (1)
133-141: Nice: clear dependency_overrides exampleThe YAML snippet concisely shows how to point to local SDK packages; aligns with the path-based strategy in this PR.
docs/PROJECT_SETUP.md (1)
47-47: No remaining cross-link updates needed for “Build release version”Search across the docs directory found only the entry in docs/PROJECT_SETUP.md (line 47) referencing “Build release version.” There are no other internal documents linking to this step, so the renumbering is confirmed and no further updates are required.
docs/CLONE_REPOSITORY.md (1)
59-63: LGTM on cross-links“Next Steps” correctly directs users to Project Setup and SDK Submodule Management. No action needed.
pubspec.yaml (1)
195-213: CI workflows and path overrides verifiedAll checks passed:
- The
sdksubmodule is declared in.gitmodulesat pathsdk.- Every GitHub Actions workflow uses
actions/checkout@v4withsubmodules: recursive, ensuringpub getcan resolve the local SDK packages.- No remaining non-commented Git or hosted references for the internal SDK packages were found in
pubspec.yaml.
- Set sdk submodule to track dev with update=checkout and on-demand fetch - Document explicit update flow and CI pinned behavior - Ensure Docker builds initialize submodule to pinned commit
…into chore/sdk-as-submodule # Conflicts: # .github/scripts/roll_sdk_packages.sh # packages/komodo_ui_kit/pubspec.lock # packages/komodo_ui_kit/pubspec.yaml # pubspec.lock # pubspec.yaml
There was a problem hiding this comment.
LGTM, thanks for the further clarifications and improvements! The failing builds are expected and awaiting fixes in #3109
spoiler
CharlVS
left a comment
CharlVS
left a comment
There was a problem hiding this comment.
Nice—this submodules setup should speed things up across SDK & KW. Less overhead than path refs and pub.dev deployments. Approved.
spoiler
🎯 Purpose & Benefits
This PR migrates from pub.dev/git dependency references to a git submodule approach for the Komodo DeFi SDK Flutter packages, providing development workflow improvements.
🔧 Developer Impact
New Clone Process:
git clone --recurse-submodules https://github.com/KomodoPlatform/komodo-wallet.git # OR git clone https://github.com/KomodoPlatform/komodo-wallet.git git submodule update --init --recursiveSDK Development Workflow:
flutter pub get && flutter build📋 Changes Summary
Dependency Management:
Documentation:
--recurse-submodulesinstructionsCI/CD Integration:
submodules: recursivecheckoutSummary by CodeRabbit
Chores
Build
Documentation