Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThe PR transitions the monorepo from a separate Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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)
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 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #560 +/- ##
==========================================
- Coverage 74.33% 74.31% -0.02%
==========================================
Files 197 197
Lines 9616 9617 +1
==========================================
- Hits 7148 7147 -1
- Misses 2468 2470 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/ndk/lib/domain_layer/usecases/cashu/cashu.dart (1)
537-541: Remove duplicate persistence in pending-state transition.Line 537 already persists via
_addAndSavePendingTransaction(...); Line 540 writes the same transaction again.♻️ Proposed fix
// update pending transactions await _addAndSavePendingTransaction(pendingTransaction); - // save pending state to cache - await _walletsRepo.saveTransactions([pendingTransaction]); - yield pendingTransaction;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ndk/lib/domain_layer/usecases/cashu/cashu.dart` around lines 537 - 541, The code currently persists the same pending transaction twice: first via _addAndSavePendingTransaction(pendingTransaction) and then again via _walletsRepo.saveTransactions([pendingTransaction]); remove the redundant call to _walletsRepo.saveTransactions so the pending-state transition is only saved by _addAndSavePendingTransaction, and verify _addAndSavePendingTransaction handles all required persistence and cache updates (adjust that method if additional save behavior is needed).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pubspec.yaml`:
- Around line 70-75: The YAML block defining command.version.hooks is
mis-indented under scripts; move the entire command: block (including version:,
hooks:, preCommit:) up two spaces so that command becomes a direct child of the
melos root (a sibling of scripts) rather than nested under scripts; ensure the
preCommit hook (melos exec ... and git add ...) remains unchanged and still
references the ndk scope and packages/ndk/lib/src/version.dart.
---
Nitpick comments:
In `@packages/ndk/lib/domain_layer/usecases/cashu/cashu.dart`:
- Around line 537-541: The code currently persists the same pending transaction
twice: first via _addAndSavePendingTransaction(pendingTransaction) and then
again via _walletsRepo.saveTransactions([pendingTransaction]); remove the
redundant call to _walletsRepo.saveTransactions so the pending-state transition
is only saved by _addAndSavePendingTransaction, and verify
_addAndSavePendingTransaction handles all required persistence and cache updates
(adjust that method if additional save behavior is needed).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6f23c454-0d0b-43b5-a523-004072e4a04f
⛔ Files ignored due to path filters (4)
packages/bip32_keys/pubspec.lockis excluded by!**/*.lockpackages/ndk/pubspec.lockis excluded by!**/*.lockpackages/ndk_cache_manager_test_suite/pubspec.lockis excluded by!**/*.lockpackages/objectbox/pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (32)
.github/workflows/tests.yamlmelos.yamlpackages/bip32_keys/pubspec.yamlpackages/drift/pubspec.yamlpackages/ndk/lib/data_layer/repositories/wallets/wallets_repo_stub.dartpackages/ndk/lib/domain_layer/entities/cashu/cashu_mint_info.dartpackages/ndk/lib/domain_layer/entities/wallet/providers/nwc/nwc_wallet.dartpackages/ndk/lib/domain_layer/entities/wallet/wallet_provider.dartpackages/ndk/lib/domain_layer/repositories/cashu_key_derivation.dartpackages/ndk/lib/domain_layer/repositories/wallets_repo.dartpackages/ndk/lib/domain_layer/usecases/cashu/cashu.dartpackages/ndk/lib/domain_layer/usecases/nwc/nwc.dartpackages/ndk/lib/domain_layer/usecases/relay_sets_engine.dartpackages/ndk/lib/presentation_layer/init.dartpackages/ndk/pubspec.yamlpackages/ndk/test/broadcast/broadcast_test.dartpackages/ndk/test/cashu/cashu_fund_test.dartpackages/ndk/test/cashu/cashu_receive_test.dartpackages/ndk/test/mocks/mock_relay.dartpackages/ndk/test/verifiers/rust_event_verifier_test.dartpackages/ndk_cache_manager_test_suite/pubspec.yamlpackages/ndk_flutter/lib/data_layer/repositories/signers/amber_event_signer.dartpackages/ndk_flutter/lib/ndk_method_channel.dartpackages/ndk_flutter/lib/widgets/locale_switcher/n_locale_switcher.dartpackages/ndk_flutter/lib/widgets/wallets/n_wallet_card.dartpackages/ndk_flutter/pubspec.yamlpackages/nip07_event_signer/pubspec.yamlpackages/objectbox/lib/data_layer/db/object_box/schema/db_metadata.dartpackages/objectbox/lib/objectbox.g.dartpackages/objectbox/pubspec.yamlpackages/objectbox/test/data_layer/cache_manager/objectbox_cache_manager_test.dartpubspec.yaml
💤 Files with no reviewable changes (6)
- packages/ndk/lib/domain_layer/repositories/cashu_key_derivation.dart
- packages/objectbox/test/data_layer/cache_manager/objectbox_cache_manager_test.dart
- packages/ndk_flutter/lib/widgets/locale_switcher/n_locale_switcher.dart
- packages/ndk/test/cashu/cashu_receive_test.dart
- packages/ndk/lib/domain_layer/entities/wallet/wallet_provider.dart
- melos.yaml
| command: | ||
| version: | ||
| hooks: | ||
| preCommit: | | ||
| melos exec --fail-fast --scope="ndk" -- "dart run build_runner build --delete-conflicting-outputs" | ||
| git add packages/ndk/lib/src/version.dart |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Melos 7, when configuring via pubspec.yaml, should command.version.hooks.preCommitbe nested undermelos(as a sibling ofscripts) or under melos.scripts?
💡 Result:
In Melos 7, when configuring via pubspec.yaml, command.version.hooks.preCommit should be nested under melos as a sibling of scripts, specifically under melos.command.version.hooks.preCommit. This structure follows the pattern shown consistently in official Melos documentation for command-specific configurations: melos: scripts: # custom scripts here command: version: hooks: preCommit: | # script content The command key is a direct child of melos, parallel to scripts. Within command.version.hooks, preCommit is defined for the version command's lifecycle hook that runs before the version commit is created. This is confirmed by multiple official sources including the Version Command documentation (explicit YAML example), Publish Command (similar command.publish.hooks structure), Configuration Overview (shows melos.command.version), and Automated Releases guide (references command/version/hooks/preCommit).
Citations:
- 1: https://melos.invertase.dev/configuration/overview
- 2: https://melos.invertase.dev/commands/publish
- 3: https://melos.invertase.dev/~522/commands/version
- 4: https://melos.invertase.dev/~259/commands/version
- 5: https://melos.invertase.dev/~526/configuration/overview
- 6: https://melos.invertase.dev/commands/version
- 7: https://melos.invertase.dev/guides/automated-releases
🏁 Script executed:
# Find and read the pubspec.yaml file around lines 70-75
git ls-files | grep -E "pubspec\.yaml$"Repository: relaystr/ndk
Length of output: 1875
command.version.hooks is mis-indented under scripts.
In Melos 7, command must be a direct sibling of scripts under melos, not nested within it. As currently configured, the version hook will not be applied. Move command to align with scripts (reduce indentation by 2 spaces).
🛠️ Proposed fix
melos:
scripts:
@@
- command:
- version:
- hooks:
- preCommit: |
- melos exec --fail-fast --scope="ndk" -- "dart run build_runner build --delete-conflicting-outputs"
- git add packages/ndk/lib/src/version.dart
+ command:
+ version:
+ hooks:
+ preCommit: |
+ melos exec --fail-fast --scope="ndk" -- "dart run build_runner build --delete-conflicting-outputs"
+ git add packages/ndk/lib/src/version.dart📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| command: | |
| version: | |
| hooks: | |
| preCommit: | | |
| melos exec --fail-fast --scope="ndk" -- "dart run build_runner build --delete-conflicting-outputs" | |
| git add packages/ndk/lib/src/version.dart | |
| command: | |
| version: | |
| hooks: | |
| preCommit: | | |
| melos exec --fail-fast --scope="ndk" -- "dart run build_runner build --delete-conflicting-outputs" | |
| git add packages/ndk/lib/src/version.dart |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pubspec.yaml` around lines 70 - 75, The YAML block defining
command.version.hooks is mis-indented under scripts; move the entire command:
block (including version:, hooks:, preCommit:) up two spaces so that command
becomes a direct child of the melos root (a sibling of scripts) rather than
nested under scripts; ensure the preCommit hook (melos exec ... and git add ...)
remains unchanged and still references the ndk scope and
packages/ndk/lib/src/version.dart.
dart workspaces
https://pub.dev/packages/melos#migrate-to-melos-7xx
run
melos cleanto delete allpubspec_overrides.yamlentries (no longer needed)benefits:
Summary by CodeRabbit