Release @yellow-org/sdk and @yellow-org/sdk-compat#598
Release @yellow-org/sdk and @yellow-org/sdk-compat#598
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughRebrands TS packages from Changes
Sequence Diagram(s)sequenceDiagram
participant App as Client/App
participant Compat as "@yellow-org/sdk-compat"
participant SDK as "@yellow-org/sdk"
participant Chain as Blockchain/RPC
App->>Compat: signChannelSessionKeyState(state)
Compat->>SDK: toSessionKeyQuorumSignature(state.signature)
SDK-->>Compat: quorumSignature
Compat->>SDK: submitChannelSessionKeyState(state, quorumSignature)
SDK->>Chain: broadcast signed session key state
Chain-->>SDK: txReceipt
SDK-->>Compat: txReceipt
Compat-->>App: result / getLastChannelKeyStates()
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
Summary of ChangesHello, 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 primarily focuses on a significant rebranding effort, transitioning the SDK packages from Highlights
Changelog
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
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@sdk/ts/package.json`:
- Line 23: The package.json "test:types" npm script currently runs "npm run
validate", but the "validate" script was removed causing the script to fail;
update the package.json by either removing the "test:types" entry or changing
its command to a valid script such as "npm run typecheck" (or whichever existing
type-checking script you maintain) so the "test:types" script points to an
actual script name; locate the "test:types" key in package.json and replace "npm
run validate" with the correct target (e.g., "npm run typecheck") or delete the
"test:types" script if it's no longer needed.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
sdk/ts-compat/package-lock.jsonis excluded by!**/package-lock.jsonsdk/ts/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
sdk/ts-compat/README.mdsdk/ts-compat/package.jsonsdk/ts-compat/src/client.tssdk/ts-compat/src/config.tssdk/ts/README.mdsdk/ts/package.json
| "lint": "eslint src --ext .ts", | ||
| "typecheck": "tsc --noEmit", | ||
| "test": "jest", | ||
| "test:types": "npm run validate", |
There was a problem hiding this comment.
Broken script reference: test:types calls removed validate script.
The test:types script runs npm run validate, but according to the AI summary, the validate script was removed. This will cause npm run test:types to fail.
🐛 Proposed fix: remove or update the broken script
Either remove the script entirely:
- "test:types": "npm run validate",Or update it to run typecheck instead:
- "test:types": "npm run validate",
+ "test:types": "npm run typecheck",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sdk/ts/package.json` at line 23, The package.json "test:types" npm script
currently runs "npm run validate", but the "validate" script was removed causing
the script to fail; update the package.json by either removing the "test:types"
entry or changing its command to a valid script such as "npm run typecheck" (or
whichever existing type-checking script you maintain) so the "test:types" script
points to an actual script name; locate the "test:types" key in package.json and
replace "npm run validate" with the correct target (e.g., "npm run typecheck")
or delete the "test:types" script if it's no longer needed.
There was a problem hiding this comment.
Code Review
This pull request appears to be a rebranding effort, renaming packages from @erc7824/nitrolite to @yellow-org/sdk. The changes primarily involve updating package names, dependencies, and documentation. I've identified a few critical and high-severity issues related to inconsistencies between package.json and package-lock.json files, which could disrupt the build and publishing processes. Additionally, I have a suggestion to improve the wording in a README file for better clarity and professionalism.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
sdk/ts-compat/README.md (2)
188-191:⚠️ Potential issue | 🟡 Minor
walletis undefined in the direct v1 client snippetThe example references
walletwithout defining it. The parameter should be the wallet address extracted from thewalletClientthat was passed toNitroliteClient.create().Suggested doc fix
const v1Client = client.innerClient; -await v1Client.getHomeChannel(wallet, 'usdc'); +await v1Client.getHomeChannel(walletClient.account!.address, 'usdc');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/ts-compat/README.md` around lines 188 - 191, The example uses an undefined variable `wallet` when calling `v1Client.getHomeChannel`; update the snippet to pass the wallet address obtained from the `walletClient` that was passed into `NitroliteClient.create()` (i.e., extract the address from the `walletClient` instance and pass that value to `client.innerClient.getHomeChannel`), referencing `client.innerClient`, `getHomeChannel`, `walletClient`, and `NitroliteClient.create()` to locate where to change the code.
71-75:⚠️ Potential issue | 🟡 MinorAdd missing
Addresstype import to the deposit exampleThe code snippet at lines 71-75 uses
as Addressbut doesn't import it. Since@yellow-org/sdk-compatdoesn't re-exportAddress, it must be imported directly fromviemfor the example to compile.Suggested fix
import { NitroliteClient, blockchainRPCsFromEnv } from '@yellow-org/sdk-compat'; +import type { Address } from 'viem';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/ts-compat/README.md` around lines 71 - 75, The README deposit example uses a type assertion "as Address" but doesn't import Address; update the example to import Address from viem and ensure the snippet includes that import so the call to client.deposit(tokenAddress as Address, amount) type-checks; specifically, add an import for Address from "viem" near the top of the example and keep the tokenAddress/amount and client.deposit usage unchanged.
🧹 Nitpick comments (1)
sdk/ts-compat/README.md (1)
262-284: Make the two error-handling snippets self-contained
AllowanceErroris imported in the first snippet but used in the second snippet. This is easy to miscopy.Suggested doc fix
-import { getUserFacingMessage, AllowanceError } from '@yellow-org/sdk-compat'; +import { getUserFacingMessage } from '@yellow-org/sdk-compat';+import { NitroliteClient, AllowanceError } from '@yellow-org/sdk-compat'; try { await client.deposit(token, amount); } catch (err) { const typed = NitroliteClient.classifyError(err);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/ts-compat/README.md` around lines 262 - 284, The two examples are not self-contained because AllowanceError is only imported in the first snippet but referenced in the second; update the second snippet that uses NitroliteClient.classifyError and AllowanceError to include the necessary import (or show a combined import block) so readers can copy it directly; specifically ensure the second snippet includes imports for AllowanceError (and getUserFacingMessage if used), references NitroliteClient.classifyError and client.deposit as shown, and demonstrates checking "instanceof AllowanceError" with a clear, copy-paste-ready import line.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@sdk/ts-compat/README.md`:
- Around line 188-191: The example uses an undefined variable `wallet` when
calling `v1Client.getHomeChannel`; update the snippet to pass the wallet address
obtained from the `walletClient` that was passed into `NitroliteClient.create()`
(i.e., extract the address from the `walletClient` instance and pass that value
to `client.innerClient.getHomeChannel`), referencing `client.innerClient`,
`getHomeChannel`, `walletClient`, and `NitroliteClient.create()` to locate where
to change the code.
- Around line 71-75: The README deposit example uses a type assertion "as
Address" but doesn't import Address; update the example to import Address from
viem and ensure the snippet includes that import so the call to
client.deposit(tokenAddress as Address, amount) type-checks; specifically, add
an import for Address from "viem" near the top of the example and keep the
tokenAddress/amount and client.deposit usage unchanged.
---
Nitpick comments:
In `@sdk/ts-compat/README.md`:
- Around line 262-284: The two examples are not self-contained because
AllowanceError is only imported in the first snippet but referenced in the
second; update the second snippet that uses NitroliteClient.classifyError and
AllowanceError to include the necessary import (or show a combined import block)
so readers can copy it directly; specifically ensure the second snippet includes
imports for AllowanceError (and getUserFacingMessage if used), references
NitroliteClient.classifyError and client.deposit as shown, and demonstrates
checking "instanceof AllowanceError" with a clear, copy-paste-ready import line.
48b5361 to
569d221
Compare
Summary by CodeRabbit
New Features
Documentation
Chores