Skip to content

Conversation

@transphorm
Copy link
Member

@transphorm transphorm commented Sep 30, 2025

Summary by CodeRabbit

  • Refactor

    • Analytics event tracking is now synchronous; remove any awaits in integrations.
    • A public field was removed from the proving store’s state; update dependent code accordingly.
  • Chores

    • Updated Android NDK version in the demo app to improve native build compatibility.
    • CI workflows updated to also run for changes in the mobile SDK package.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 30, 2025

Walkthrough

Synchronized trackEvent in the mobile SDK client from async Promise to sync void using internal _adapters.analytics; removed selfApp from the proving store’s public state; bumped demo Android ndkVersion; extended CI workflow pull_request paths to include packages/mobile-sdk-alpha/**.

Changes

Cohort / File(s) Summary
SDK client event tracking
packages/mobile-sdk-alpha/src/client.ts
trackEvent changed from async (…): Promise<void> to synchronous (...): void; availability check and invocation switched from adapters.analytics to _adapters.analytics; no await/early Promise return.
Proving store state shape
packages/mobile-sdk-alpha/src/proving/provingMachine.ts
Removed selfApp from the ProvingState initial state exposed by useProvingStore; internal access still available via selfClient.getSelfAppState().
Demo Android build config
packages/mobile-sdk-demo/android/build.gradle
Bumped ndkVersion from 27.0.11718014 to 27.0.12077973.
CI workflow triggers
.github/workflows/mobile-e2e.yml, .github/workflows/mobile-sdk-demo-ci.yml
Added packages/mobile-sdk-alpha/** to pull_request path filters so changes in the SDK now trigger these workflows.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor App
  participant Client as MobileSDKClient
  participant Analytics as AnalyticsAdapter

  rect rgb(235,245,255)
  note over App,Client: Previous (async) flow
  App->>Client: trackEvent(event, payload)
  alt adapters.analytics available
    Client->>Analytics: trackEvent(event, payload)
    Analytics-->>Client: Promise resolved
    Client-->>App: Promise<void> (awaitable)
  else missing adapter
    Client-->>App: Promise<void> (early return)
  end
  end

  Note over App,Client: ———

  rect rgb(240,255,240)
  note over App,Client: New (sync) flow
  App->>Client: trackEvent(event, payload)
  alt _adapters.analytics present
    Client->>Analytics: trackEvent(event, payload)   %% not awaited
    Client-->>App: void (non-async)
  else missing adapter
    Client-->>App: void (no-op)
  end
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

codex

Suggested reviewers

  • aaronmgdr
  • remicolin

Poem

Synchronous footsteps, events set free,
A leaner proving state, tidy as can be.
NDK steps onward, builds hum in tune,
CI now listens when SDKs change soon.
Small edits, bright ripple — deploy by the moon. 🌙

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Title Check ⚠️ Warning The title “small tweaks to bump build v2.6.8” highlights only a version bump and omits the key refactoring of the trackEvent method to a synchronous signature, the removal of the selfApp property from the proving store, and the CI workflow trigger updates, making it misleading and not representative of the main changes. Please revise the title to concisely summarize the primary changes—such as “Refactor trackEvent to synchronous, remove selfApp from proving store, bump NDK version, and extend CI triggers”—to accurately reflect the PR’s content.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch justin/chore-bump-build-for-auto-deploy-v268

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.

❤️ Share
🧪 Early access (Sonnet 4.5): enabled

We are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience.

Note:

  • Public repositories are always opted into early access features.
  • You can enable or disable early access features from the CodeRabbit UI or by updating the CodeRabbit configuration file.

Comment @coderabbitai help to get the list of available commands and usage tips.

@transphorm transphorm changed the title small tweaks to bump build small tweaks to bump build v2.6.8 Sep 30, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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)
.github/workflows/mobile-e2e.yml (2)

236-239: Direct cache usage violates coding guidelines.

This workflow uses actions/cache@v4 directly instead of the shared composite caching actions from .github/actions/. Per the coding guidelines, caching should use the shared composite actions (like cache-yarn, cache-gradle, cache-pods) for consistency across workflows.

As per coding guidelines.

Consider creating a shared composite action for Maestro caching (e.g., .github/actions/cache-maestro) and using it here instead of calling actions/cache directly.


282-288: Direct cache usage violates coding guidelines.

This DerivedData cache uses actions/cache@v4 directly instead of a shared composite caching action. Per the coding guidelines, all caching should use shared composite actions from .github/actions/ to maintain consistency and allow centralized cache key management.

As per coding guidelines.

Consider creating a shared composite action for DerivedData caching (e.g., .github/actions/cache-derived-data) that accepts parameters for path, Xcode version, and lock file hashes.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 76bed2f and d7dd052.

📒 Files selected for processing (2)
  • .github/workflows/mobile-e2e.yml (1 hunks)
  • .github/workflows/mobile-sdk-demo-ci.yml (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
.github/workflows/**/*.{yml,yaml}

📄 CodeRabbit inference engine (AGENTS.md)

.github/workflows/**/*.{yml,yaml}: In GitHub Actions workflows, use shared composite caching actions from .github/actions (cache-yarn, cache-bundler, cache-gradle, cache-pods)
Do not call actions/cache directly; rely on the shared composite caching actions
When using cache actions, optionally pass cache-version (often with GH_CACHE_VERSION and tool version) for stable keys

Files:

  • .github/workflows/mobile-e2e.yml
  • .github/workflows/mobile-sdk-demo-ci.yml
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: build-deps
  • GitHub Check: android-build-test
  • GitHub Check: e2e-ios
  • GitHub Check: analyze-ios
  • GitHub Check: analyze-android
🔇 Additional comments (2)
.github/workflows/mobile-sdk-demo-ci.yml (1)

6-6: LGTM! Path trigger correctly includes mobile-sdk-alpha.

The addition of packages/mobile-sdk-alpha/** to the workflow triggers ensures that changes to the new alpha SDK package will properly trigger CI builds for the demo app.

.github/workflows/mobile-e2e.yml (1)

27-27: LGTM! Path trigger correctly includes mobile-sdk-alpha.

The addition of packages/mobile-sdk-alpha/** to the workflow triggers ensures that changes to the new alpha SDK package will properly trigger end-to-end tests, maintaining consistency with the demo CI workflow.

@transphorm transphorm merged commit 9ebb916 into dev Sep 30, 2025
27 checks passed
@transphorm transphorm deleted the justin/chore-bump-build-for-auto-deploy-v268 branch September 30, 2025 02:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants