-
Notifications
You must be signed in to change notification settings - Fork 25
feat: add configFetcher setting #630
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
WalkthroughThe PR bumps all package versions from 1.1.20-alpha.2 to 1.1.20 across the monorepo. It also adds a configurable multi-step config-fetch strategy to DataManager with logging and firmware type enrichment, plus an optional configFetcher callback to ConnectSettings. Changes
Sequence DiagramsequenceDiagram
participant App as Application
participant DM as DataManager
participant CF as configFetcher<br/>(if provided)
participant HTTP as Axios HTTP
participant Log as Logger
App->>DM: load()
activate DM
alt configFetcher provided
DM->>CF: fetch config
activate CF
CF-->>DM: config or error
deactivate CF
DM->>Log: log attempt result
else no configFetcher
DM->>Log: debug: no custom fetcher
end
alt configFetcher failed or not provided
DM->>HTTP: GET config (axios)
activate HTTP
HTTP-->>DM: config or error
deactivate HTTP
DM->>Log: log axios result
end
alt config obtained
DM->>DM: populate deviceMap,<br/>enrich firmware
DM->>Log: debug: config loaded
else all attempts failed
DM->>Log: warn: using defaults
end
DM-->>App: ready
deactivate DM
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to data retention organization setting Disabled knowledge base sources:
📒 Files selected for processing (17)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (21)
Comment |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/data-manager/DataManager.ts (1)
381-383: Use the module logger for consistency.Line 382 uses
console.errorwhile the rest of the file usesLog. Stick to one approach.🔎 Proposed fix
} catch (error) { - console.error(`Error enriching firmware field "${field}":`, error); + Log.error(`[DataConfig] Error enriching firmware field "${field}":`, error); }
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (17)
packages/connect-examples/electron-example/package.json(2 hunks)packages/connect-examples/expo-example/package.json(2 hunks)packages/connect-examples/expo-playground/package.json(2 hunks)packages/core/package.json(2 hunks)packages/core/src/data-manager/DataManager.ts(4 hunks)packages/core/src/types/settings.ts(1 hunks)packages/hd-ble-sdk/package.json(2 hunks)packages/hd-common-connect-sdk/package.json(2 hunks)packages/hd-transport-electron/package.json(2 hunks)packages/hd-transport-emulator/package.json(2 hunks)packages/hd-transport-http/package.json(2 hunks)packages/hd-transport-lowlevel/package.json(2 hunks)packages/hd-transport-react-native/package.json(2 hunks)packages/hd-transport-web-device/package.json(2 hunks)packages/hd-transport/package.json(1 hunks)packages/hd-web-sdk/package.json(2 hunks)packages/shared/package.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: lint (22)
- GitHub Check: build (22)
🔇 Additional comments (21)
packages/core/src/types/settings.ts (1)
35-35: LGTM!Clean type definition. The optional
configFetchercallback withPromise<RemoteConfigResponse | null>return type allows graceful fallback when the custom fetcher fails or isn't provided.packages/core/src/data-manager/DataManager.ts (3)
32-33: LGTM!Module-level logger is appropriate for this static class pattern.
45-62: LGTM!Good defensive design. The
Readonly<Record<...>>ensures compile-time coverage of all firmware fields. The runtime fallback toUniversaladds extra safety.
394-451: Clean staged fetch design.The fallback from
configFetcher→axios→ default config is clear and well-logged. ThefetchMethodtracking adds useful context to logs.packages/hd-transport/package.json (1)
3-3: LGTM: Clean version bump to stable release.The alpha prerelease suffix removal is consistent with the PR's monorepo-wide upgrade to 1.1.20.
packages/shared/package.json (1)
3-3: LGTM: Version bump is consistent.Matches the monorepo-wide migration to stable 1.1.20.
packages/hd-transport-lowlevel/package.json (1)
3-3: LGTM: Version and dependencies aligned.Both package version and internal dependencies correctly updated to stable 1.1.20.
Also applies to: 22-23
packages/hd-transport-emulator/package.json (1)
3-3: LGTM: Consistent upgrade across version and dependencies.Package and internal dependencies correctly moved to 1.1.20.
Also applies to: 27-28
packages/core/package.json (1)
3-3: LGTM: Core package upgrade is properly aligned.Version and dependencies consistently updated to stable 1.1.20.
Also applies to: 28-29
packages/connect-examples/electron-example/package.json (1)
5-5: LGTM: Example package correctly updated.Version and hd-transport-electron dependency aligned with stable 1.1.20 release.
Also applies to: 25-25
packages/hd-transport-react-native/package.json (1)
3-3: LGTM: React Native transport package aligned.Version and internal dependencies correctly updated to 1.1.20.
Also applies to: 22-23
packages/connect-examples/expo-playground/package.json (2)
3-3: LGTM: Playground dependencies consistently upgraded.Version and all SDK dependencies properly aligned with stable 1.1.20.
Also applies to: 20-22
20-22: Version 1.1.20 is available for publishing.Checked npm registry: v1.1.20 does not exist for @onekeyfe/hd-common-connect-sdk, @onekeyfe/hd-core, or @onekeyfe/hd-shared. No publish conflicts. npm automatically checks for security advisories during publication.
packages/hd-transport-electron/package.json (1)
3-3: Version bump looks good.The package and its internal dependencies are correctly updated from alpha prerelease to stable 1.1.20.
Also applies to: 28-30
packages/hd-web-sdk/package.json (1)
3-3: Version alignment is correct.Package and dependencies updated consistently to stable 1.1.20.
Also applies to: 24-27
packages/connect-examples/expo-example/package.json (1)
3-3: Example app dependencies updated correctly.All internal SDK dependencies aligned to stable 1.1.20.
Also applies to: 22-25
packages/hd-ble-sdk/package.json (1)
3-3: BLE SDK version bump is consistent.Package and dependencies correctly updated to 1.1.20.
Also applies to: 23-25
packages/hd-transport-http/package.json (1)
3-3: Transport HTTP version updated correctly.Internal dependencies aligned to 1.1.20.
Also applies to: 27-28
packages/hd-common-connect-sdk/package.json (1)
3-3: Connect SDK dependencies aligned.All six internal dependencies correctly bumped to 1.1.20.
Also applies to: 23-28
packages/hd-transport-web-device/package.json (2)
3-3: Web device transport versions aligned.Runtime and dev dependencies correctly updated to 1.1.20.
Also applies to: 23-24, 27-27
1-31: Version bump and configFetcher feature verified.All monorepo packages are correctly updated to version 1.1.20 (no alpha versions remain). The configFetcher feature is implemented in packages/core with both type definitions in settings.ts and fallback logic in DataManager.ts. The PR changes are confirmed complete.
Summary by CodeRabbit
Release Notes
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.