-
Notifications
You must be signed in to change notification settings - Fork 557
[SDK] Fix: Deprecate test mode #7396
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
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe change adds a JSDoc Changes
Suggested labels
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
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
🧹 Nitpick comments (2)
packages/thirdweb/src/react/core/hooks/connection/ConnectButtonProps.ts (2)
80-83
: Enhance deprecation annotation with context
The@deprecated
tag is correctly applied tobuyWithCrypto.testMode
, but it lacks details such as the deprecation version, removal timeline, or recommended alternative. Please enrich the JSDoc, for example:/** * @deprecated since vX.Y — testMode is no longer supported and will be removed in vZ. Use <alternativeOption> instead. */ testMode?: boolean;
101-104
: Apply consistent deprecation messaging for fiat flow
Mirror the enhanced deprecation notice forbuyWithFiat.testMode
to ensure both payment options provide the same guidance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/thirdweb/src/react/core/hooks/connection/ConnectButtonProps.ts
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: Unit Tests
- GitHub Check: Analyze (javascript)
size-limit report 📦
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7396 +/- ##
==========================================
- Coverage 52.20% 52.19% -0.01%
==========================================
Files 945 945
Lines 63578 63578
Branches 4229 4227 -2
==========================================
- Hits 33192 33186 -6
- Misses 30279 30285 +6
Partials 107 107
🚀 New features to boost your workflow:
|
PR-Codex overview
This PR introduces deprecation notices for the
testMode
property in two types within theConnectButtonProps.ts
file, signaling that these properties should no longer be used.Detailed summary
@deprecated
comment to thetestMode
property in the first instance.@deprecated
comment to thetestMode
property in thebuyWithFiat
options.Summary by CodeRabbit