chore: add a rn-sdk beta release script#2141
Merged
santhoshvai merged 3 commits intofeat/callkit-telecom-integrationfrom Feb 27, 2026
Merged
chore: add a rn-sdk beta release script#2141santhoshvai merged 3 commits intofeat/callkit-telecom-integrationfrom
santhoshvai merged 3 commits intofeat/callkit-telecom-integrationfrom
Conversation
|
Contributor
|
Caution Review failedFailed to post review comments 📝 WalkthroughWalkthroughAdds a new React Native Calling package ( Changes
Sequence DiagramsequenceDiagram
participant Push as Push Provider
participant SDK as Stream Video SDK (JS)
participant Callingx as Callingx Native Module
participant OS as Platform (CallKit/Telecom/Notification)
participant App as App UI / Client
Push->>SDK: deliver incoming push (call_cid, payload)
SDK->>Callingx: callingx.displayIncomingCall(callId, ...)
Callingx->>OS: report incoming call (CallKit/Telecom) / post notification
OS->>Callingx: user action (answer / reject)
Callingx->>SDK: emit event (answerCall / endCall / audioSession)
SDK->>App: notify app state (onNewEvent)
App->>SDK: app-level call handling (start/stop)
SDK->>Callingx: callingx.startCall / callingx.endCall(call, reason)
Callingx->>OS: update call state / end call
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
scripts/release-rn-sdk-beta.mjs (1)
391-400: Consider reusing monorepo build entrypoints in release flow.
runBuild()directly invokes per-workspace builds; using shared root scripts for RN deps/SDK reduces drift and keeps release automation aligned with existing dependency-order guarantees.Based on learnings: Use monorepo build commands (
yarn build:react-native:deps,yarn build:react-native:sdk) to ensure proper dependency ordering when working with workspace packages.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/release-rn-sdk-beta.mjs` around lines 391 - 400, runBuild currently calls workspace-specific build scripts directly; instead, detect when packageName corresponds to the React Native dependency or SDK workspaces and call the monorepo-level build entrypoints to preserve dependency ordering: inside runBuild (referencing function runBuild, parameter packageName, and the run(...) invocation) add a conditional that maps the RN workspaces to root commands and runs 'yarn build:react-native:deps' for the RN deps workspace and 'yarn build:react-native:sdk' for the RN SDK workspace (preserving dryRun behavior and logging), otherwise fall back to the existing 'yarn workspace <package> run build' path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/release-rn-sdk-beta.mjs`:
- Around line 422-435: The verifyPublishedVersion function should retry the npm
view call with exponential backoff instead of failing immediately; wrap the
run('npm', ['view', `${packageName}@${version}`, 'version']) call in a retry
loop (e.g., 3-5 attempts) that waits increasing intervals (e.g., 1s, 2s, 4s)
between attempts and throws the existing Error only after all attempts fail,
preserve the dryRun early return and keep the same error message when ultimately
unsuccessful.
- Around line 31-37: The parser currently blindly consumes argv[i+1] for flags
'--base-ref' and '--tag' which can incorrectly pick up another flag (e.g.,
'--tag --dry-run'); update the handling in the loop that uses variables arg,
argv, i and options to validate argv[i+1] exists and does not start with '-'
before assigning to options.baseRef or options.tag, and if invalid emit a clear
error or exit (instead of consuming the flag) so callers get an actionable
message.
---
Nitpick comments:
In `@scripts/release-rn-sdk-beta.mjs`:
- Around line 391-400: runBuild currently calls workspace-specific build scripts
directly; instead, detect when packageName corresponds to the React Native
dependency or SDK workspaces and call the monorepo-level build entrypoints to
preserve dependency ordering: inside runBuild (referencing function runBuild,
parameter packageName, and the run(...) invocation) add a conditional that maps
the RN workspaces to root commands and runs 'yarn build:react-native:deps' for
the RN deps workspace and 'yarn build:react-native:sdk' for the RN SDK workspace
(preserving dryRun behavior and logging), otherwise fall back to the existing
'yarn workspace <package> run build' path.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
package.jsonscripts/release-rn-sdk-beta.mjs
Comment on lines
+31
to
+37
| if (arg === '--base-ref') { | ||
| options.baseRef = argv[i + 1]; | ||
| i += 1; | ||
| } else if (arg === '--tag') { | ||
| options.tag = argv[i + 1]; | ||
| i += 1; | ||
| } else if (arg === '--dry-run') { |
Contributor
There was a problem hiding this comment.
Validate option values before consuming --base-ref and --tag.
If the next token is another flag (for example --tag --dry-run), it gets consumed as a value and fails later with a less actionable error.
🛠️ Proposed fix
+function readRequiredArgValue(argv, index, flag) {
+ const value = argv[index + 1];
+ if (!value || value.startsWith('-')) {
+ printHelpAndExit(1, `${flag} requires a value.`);
+ }
+ return value;
+}
+
function parseArgs(argv) {
const options = {
@@
for (let i = 0; i < argv.length; i += 1) {
const arg = argv[i];
if (arg === '--base-ref') {
- options.baseRef = argv[i + 1];
+ options.baseRef = readRequiredArgValue(argv, i, '--base-ref');
i += 1;
} else if (arg === '--tag') {
- options.tag = argv[i + 1];
+ options.tag = readRequiredArgValue(argv, i, '--tag');
i += 1;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/release-rn-sdk-beta.mjs` around lines 31 - 37, The parser currently
blindly consumes argv[i+1] for flags '--base-ref' and '--tag' which can
incorrectly pick up another flag (e.g., '--tag --dry-run'); update the handling
in the loop that uses variables arg, argv, i and options to validate argv[i+1]
exists and does not start with '-' before assigning to options.baseRef or
options.tag, and if invalid emit a clear error or exit (instead of consuming the
flag) so callers get an actionable message.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
scripts/release-rn-sdk-beta.mjs (3)
633-636: Hardcoded path for version.ts.The path
packages/react-native-sdk/src/version.tsis hardcoded. While this is stable and aligns with project conventions, consider deriving it fromrnInfo.locationfor consistency:♻️ Derive path from workspace info
backupFile( - resolve(process.cwd(), 'packages/react-native-sdk/src/version.ts'), + resolve(process.cwd(), rnInfo.location, 'src/version.ts'), backups, );Based on learnings: "Store generated version information in
src/version.ts(auto-generated viayarn copy-versionscript)".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/release-rn-sdk-beta.mjs` around lines 633 - 636, Replace the hardcoded path passed to backupFile with a path derived from rnInfo.location; instead of resolve(process.cwd(), 'packages/react-native-sdk/src/version.ts'), construct the version file path using path.join(rnInfo.location, 'src', 'version.ts') (or resolve(rnInfo.location, 'src', 'version.ts')) and pass that to backupFile so it consistently uses the workspace info (keep using the same backups variable).
463-466: Consider a more idiomatic sleep implementation.Using
Atomics.waitfor synchronous sleep is functional but uncommon. An alternative usingsetTimeoutwith promisification would be more readable:♻️ Alternative using timers/promises
+import { setTimeout as sleep } from 'node:timers/promises'; + // Block synchronously for simple retry backoff. -function sleep(ms) { - Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, ms); -} +// Note: This requires making verifyPublishedVersion async +// async function sleep(ms) { ... } or use timers/promisesIf keeping synchronous execution is important, the current implementation works fine. Alternatively,
child_process.spawnSync('sleep', [String(ms/1000)])on Unix systems.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/release-rn-sdk-beta.mjs` around lines 463 - 466, Replace the synchronous Atomics.wait sleep implementation in function sleep with an idiomatic async/promise-based approach: change the function signature to return a Promise and use setTimeout (or util.promisify(setTimeout)) to resolve after ms, update callers of sleep to await it (or adapt call sites) so behavior remains the same but code becomes more readable and non-blocking; keep the Atomics.wait version only if synchronous blocking is explicitly required.
646-651: Consider handling process interruption signals.If the user interrupts the script (Ctrl+C) mid-execution, the
finallyblock may not fully execute, leaving modifiedpackage.jsonfiles. Consider adding signal handlers to ensure cleanup:♻️ Add signal handlers for graceful cleanup
// Add near the top of main() or before try block process.on('SIGINT', () => { console.log('\nInterrupted. Restoring backups...'); if (!options.keepChanges && backups.size > 0) { restoreBackups(backups); } process.exit(130); });This ensures manifests are restored even on manual interruption.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/release-rn-sdk-beta.mjs` around lines 646 - 651, Add OS signal handlers so restoreBackups(backups) runs on interruption: register handlers for SIGINT and SIGTERM (near the top of main() or before the try block where backups and options are created) that check if !options.keepChanges && backups.size > 0 then call restoreBackups(backups) and exit with appropriate code; ensure the handlers are idempotent (no double-restore) and removed or ignored after the normal finally/cleanup completes so normal flow and process.exit paths remain unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@scripts/release-rn-sdk-beta.mjs`:
- Around line 633-636: Replace the hardcoded path passed to backupFile with a
path derived from rnInfo.location; instead of resolve(process.cwd(),
'packages/react-native-sdk/src/version.ts'), construct the version file path
using path.join(rnInfo.location, 'src', 'version.ts') (or
resolve(rnInfo.location, 'src', 'version.ts')) and pass that to backupFile so it
consistently uses the workspace info (keep using the same backups variable).
- Around line 463-466: Replace the synchronous Atomics.wait sleep implementation
in function sleep with an idiomatic async/promise-based approach: change the
function signature to return a Promise and use setTimeout (or
util.promisify(setTimeout)) to resolve after ms, update callers of sleep to
await it (or adapt call sites) so behavior remains the same but code becomes
more readable and non-blocking; keep the Atomics.wait version only if
synchronous blocking is explicitly required.
- Around line 646-651: Add OS signal handlers so restoreBackups(backups) runs on
interruption: register handlers for SIGINT and SIGTERM (near the top of main()
or before the try block where backups and options are created) that check if
!options.keepChanges && backups.size > 0 then call restoreBackups(backups) and
exit with appropriate code; ensure the handlers are idempotent (no
double-restore) and removed or ignored after the normal finally/cleanup
completes so normal flow and process.exit paths remain unchanged.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
scripts/release-rn-sdk-beta.mjs
santhoshvai
changed the base branch from
main
to
feat/callkit-telecom-integration
santhoshvai
merged commit f2f41da
into
feat/callkit-telecom-integration
19 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Documentation
Chores