Repository navigation
Use @ecency/sdk/wallets in the apps and remove the @ecency/wallets package - #2066
Conversation
Point every apps/web and apps/self-hosted import and spec mock at the SDK wallets entry instead of the @ecency/wallets shim, and drop the shim from transpilePackages. Refs #2062
The wallets code ships as @ecency/sdk/wallets, so delete packages/wallets and drop it from the app dependencies, Dockerfiles, the packages and self-hosted workflows, the auto-changeset package list and labels, the root scripts and the lockfile. scryptlib leaves allowBuilds: nothing in the lockfile needs it any more. A new SDK spec fails if app source, a manifest, a Dockerfile, a workflow or the lockfile references the removed package again. Refs #2062
List three publishable packages, describe wallet code as the @ecency/sdk/wallets entry, note that @ecency/wallets is deprecated on npm, point the test docs at the @ecency/sdk/wallets global mock and fix the external chain list (BTC, ETH, BNB, SOL). Refs #2062
…label The fork release validator still allowed packages/wallets dist, manifest and changelog paths, so a release artifact could recreate the removed package; a test case now covers it. The javascript label that Dependabot applies is added to labels.yml, so the label sync triggered by this change does not delete it.
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 3 minutes. View limit details
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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 325584671b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return REMOVED.filter((needle) => text.includes(needle)).map((needle) => `${f}: ${needle}`); | ||
| }); | ||
| expect(hits).toEqual([]); |
There was a problem hiding this comment.
The new guard always fails on this commit because codeAndTooling() includes .github/scripts/test-fork-release-patch.sh, whose new negative-test fixture deliberately contains packages/wallets/dist on lines 58–59. Reproducing the scan returns [".github/scripts/test-fork-release-patch.sh: packages/wallets"], so this assertion breaks the SDK test suite even though no dependency on the removed package remains. Exclude this intentional fixture or construct its removed-package path without a literal match.
Useful? React with 👍 / 👎.
PR Summary by QodoMove app wallet imports to the SDK and remove the wallets package
AI Description
Diagram
High-Level Assessment
Files changed (64)
|
Code Review by Qodo
1. Migration docs still name the removed wallets package
|
 [Medium impact] The PR appears safe to merge; the previous test failure is fixed. SummaryBoth apps now import wallet code from
Reviews (2) · Last reviewed commit: "Build the removed wallets path in pieces..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
The new reject case wrote the removed package path literally, and the no-wallets-package guard scans .github/scripts, so the SDK suite failed. The fixture now builds the path from pieces, as the guard spec does; the guard still flags a real reference in that script.
Closes #2062 (PR B of 2; PR A was #2063).
Release labels: none
No package source changes here, so this PR needs no release.
Do not add a generic
patch,minorormajorlabel. The label workflow runs the develop copy ofauto-changeset.sh, which still listswallets. Because this PR touchespackages/wallets, a generic label would write a changeset for a package that no longer exists, andchangeset versionwould fail.What changes
Apps
apps/webandapps/self-hostedimport wallet code from@ecency/sdk/wallets. Test mocks are switched with them.Package and tooling
packages/walletsis deleted.@ecency/wallets@7.0.1, the last release, stays on npm as a re-export shim.packages.yml,self-hosted.ymland the auto-changeset label gate;auto-changeset.shand the fork release validator. Its path allowlist no longer acceptspackages/wallets, with a new test case;.gitignore,pnpm-workspace.yaml(scryptlib) and the lockfile;next.config.jstranspilePackages.@ecency/sdkalready covers the subpath.Labels
.github/labels.ymldropspatch:walletsandminor:wallets. The label sync deletes them on merge; no open PR carries them.javascript, which Dependabot applies, so the same sync does not delete it.documentation(listed in the file since January but not live) and sets thedependenciesdescription to the file's text.Docs
PUBLISHING.mdare updated.@ecency/walletson purpose: they say it is deprecated in favour of@ecency/sdk/wallets.Test plan
packages/sdk/test/no-wallets-package.spec.tsfails ifpackages/walletsexists, or if app source, a manifest, a Dockerfile, a workflow, a CI script,next.config.jsor the lockfile references@ecency/walletsorpackages/wallets. Markdown is excluded so the deprecation notes can stay.packages/wallets/distfile fails on the old allowlist and passes now.vitest run: 6367 tests.vitest run: 1443 tests.tsc --noEmit.test-auto-changeset.shandtest-fork-release-patch.sh.pnpm install --frozen-lockfile.