-
Notifications
You must be signed in to change notification settings - Fork 11
Merging Expo and RN docs, with expo specific instructions #75
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 GitHub.
|
|
madhavanmalolan seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
WalkthroughRemoves Expo-specific docs and metadata, updates docs navigation to drop Expo, restructures React Native installation docs to separate Expo vs. non-Expo flows with expanded setup steps, and fixes minor syntax issues in zk-fetch quickstart code snippets. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing touches🧪 Generate unit tests
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 |
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
content/docs/zkfetch/quickstart.mdx (3)
21-23: Incomplete sentence in Callout.
Ends abruptly; finalize the thought.Apply:
- The following tutorial is based on a Node.js + The following tutorial targets a Node.js environment.
150-156: Invalid JS syntax in responseMatches example.
'contains' | 'regex'and'<HTTP RESPONSE TEXT>' | '<REGEX>'aren’t valid JS; readers will copy-paste broken code.Replace with concrete entries:
- responseMatches: [ - { - type: 'contains' | 'regex', // type of match - value: '<HTTP RESPONSE TEXT>' | '<REGEX>', // value to match or regex to match - } - ], + responseMatches: [ + { type: 'contains', value: '<HTTP RESPONSE TEXT>' }, + { type: 'regex', value: /<REGEX>/ } + ],
216-216: Fix placeholders to be valid JS strings.
Current line won’t run when copy-pasted.-const client = new ReclaimClient(YOUR_APP_ID, YOUR_APP_SECRET); +const client = new ReclaimClient('YOUR_APP_ID', 'YOUR_APP_SECRET');content/docs/react-native/installation.mdx (3)
111-142: Use System.getenv in Gradle (Groovy/Kotlin) examples.
System.envisn’t valid in these DSLs; useSystem.getenv("VAR").-String flutterStorageUrl = System.env.FLUTTER_STORAGE_BASE_URL ?: "https://storage.googleapis.com" -String reclaimStorageUrl = System.env.RECLAIM_STORAGE_BASE_URL ?: "https://reclaim-inapp-sdk.s3.ap-south-1.amazonaws.com/android/repo" +String flutterStorageUrl = System.getenv("FLUTTER_STORAGE_BASE_URL") ?: "https://storage.googleapis.com" +String reclaimStorageUrl = System.getenv("RECLAIM_STORAGE_BASE_URL") ?: "https://reclaim-inapp-sdk.s3.ap-south-1.amazonaws.com/android/repo"-val flutterStorageUrl = System.env.FLUTTER_STORAGE_BASE_URL ?: "https://storage.googleapis.com" -val reclaimStorageUrl = System.env.RECLAIM_STORAGE_BASE_URL ?: "https://reclaim-inapp-sdk.s3.ap-south-1.amazonaws.com/android/repo" +val flutterStorageUrl = System.getenv("FLUTTER_STORAGE_BASE_URL") ?: "https://storage.googleapis.com" +val reclaimStorageUrl = System.getenv("RECLAIM_STORAGE_BASE_URL") ?: "https://reclaim-inapp-sdk.s3.ap-south-1.amazonaws.com/android/repo"
151-169: Repeat fix for alternative repositories block.
Same environment variable issue here.-String flutterStorageUrl = System.env.FLUTTER_STORAGE_BASE_URL ?: "https://storage.googleapis.com" -String reclaimStorageUrl = System.env.RECLAIM_STORAGE_BASE_URL ?: "https://reclaim-inapp-sdk.s3.ap-south-1.amazonaws.com/android/repo" +String flutterStorageUrl = System.getenv("FLUTTER_STORAGE_BASE_URL") ?: "https://storage.googleapis.com" +String reclaimStorageUrl = System.getenv("RECLAIM_STORAGE_BASE_URL") ?: "https://reclaim-inapp-sdk.s3.ap-south-1.amazonaws.com/android/repo"-val flutterStorageUrl = System.env.FLUTTER_STORAGE_BASE_URL ?: "https://storage.googleapis.com" -val reclaimStorageUrl = System.env.RECLAIM_STORAGE_BASE_URL ?: "https://reclaim-inapp-sdk.s3.ap-south-1.amazonaws.com/android/repo" +val flutterStorageUrl = System.getenv("FLUTTER_STORAGE_BASE_URL") ?: "https://storage.googleapis.com" +val reclaimStorageUrl = System.getenv("RECLAIM_STORAGE_BASE_URL") ?: "https://reclaim-inapp-sdk.s3.ap-south-1.amazonaws.com/android/repo"
213-236: Remove SDK-maintainer-only Podfile hook.
pre_install { system("... npx bob build ...") }is for the SDK repo, not app integrators; it will break consumer apps.# This is the line that you may need to add in your podfile. pod 'ReclaimInAppSdk', '~> 0.6.0' - - pre_install do |installer| - system("cd ../../ && npx bob build --target codegen") - end
🧹 Nitpick comments (7)
content/docs/zkfetch/quickstart.mdx (5)
55-62: Tighten wording for “Public endpoints”.
Improve readability; avoid fragments.-If the endpoint you want to _fetch_ and generate a proof of the response. This endpoint is public, and doesn't need any private data like auth headers/api keys. +Use this when the endpoint is public and doesn’t require private data (e.g., auth headers or API keys).
76-76: Polish the visibility note.
Minor grammar improvement.-Note : all the data in the publicOptions will be visible to them who you share the proof with (aka, verifier). +Note: Everything in publicOptions is visible to anyone you share the proof with (the verifier).
78-84: Pluralize and punctuate “private endpoint(s)”.-##### For private endpoint -If you want to _fetch_ and generate a proof of the response, but the fetch involves some private data like auth headers or api keys +##### For private endpoints +Use this when the request involves private data such as auth headers or API keys.
117-117: Fix typo: “Additonal” → “Additional”.-### Additonal Features +### Additional Features
139-139: Fix typo: “ou” → “You”.-ou can also use responseMatches and responseRedactions to match and redact the response. +You can also use responseMatches and responseRedactions to match and redact the response.content/docs/react-native/installation.mdx (2)
245-247: Use “iOS” casing.
Minor doc polish.-#### Fixing performance issues on IOS physical devices +#### Fixing performance issues on iOS physical devices
109-146: Explain non-standard repositories (Flutter/Go) in RN context.
If these repos are required, add a one-liner explaining why; if not, remove to avoid surprising RN users.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
content/docs/expo/installation.mdx(0 hunks)content/docs/expo/meta.json(0 hunks)content/docs/expo/usage.mdx(0 hunks)content/docs/meta.json(0 hunks)content/docs/react-native/installation.mdx(1 hunks)content/docs/zkfetch/quickstart.mdx(2 hunks)
💤 Files with no reviewable changes (4)
- content/docs/expo/meta.json
- content/docs/expo/installation.mdx
- content/docs/meta.json
- content/docs/expo/usage.mdx
🔇 Additional comments (7)
content/docs/zkfetch/quickstart.mdx (4)
44-45: Import fix looks good.
Correct module path and syntax.
6-6: Missing Callout import (verify).
If Callout isn’t globally registered, this will fail at build-time.If needed, add:
import { Step, Steps } from 'fumadocs-ui/components/steps'; +import { Callout } from 'fumadocs-ui/components/callout';
94-98: Header object fix looks correct.
The earlier invalid object syntax is resolved; example is now copy-pastable.
203-204: Confirm sync vs async for transformForOnchain and standardize usage.
This page uses it both with and without await; pick one based on actual API.Would you confirm whether
Reclaim.transformForOnchainreturns a Promise? If yes, addawaiteverywhere; if not, remove it everywhere.content/docs/react-native/installation.mdx (3)
16-17: Good move bringing API Key to the top.
Improves flow before platform-specific steps.
190-209: Pinning versions: verify current iOS SDK tag.
Ensure0.6.0is the latest stable; otherwise readers will pull outdated bits.Would you like me to check the latest
ReclaimInAppSdkpod version and update the examples accordingly?
292-294: Confirm CONTRIBUTING link target.
Relative link may not resolve on the docs site.Consider linking to the repo URL, e.g.:
-See the [contributing guide](CONTRIBUTING.md) ... +See the [contributing guide](https://github.com/reclaimprotocol/docs/blob/main/CONTRIBUTING.md) ...
| ``` | ||
| # For online builds | ||
| npx eas-cli build --profile development | ||
| # For local builds | ||
| npx eas-cli build --profile development --local | ||
| ``` |
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.
Incorrect EAS CLI invocation.
The bin is eas, not eas-cli. npx eas-cli will fail.
-# For online builds
-npx eas-cli build --profile development
+# For online builds
+npx eas build --profile development
...
-# For local builds
-npx eas-cli build --profile development --local
+# For local builds
+npx eas build --profile development --local📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ``` | |
| # For online builds | |
| npx eas-cli build --profile development | |
| # For local builds | |
| npx eas-cli build --profile development --local | |
| ``` | |
| # For online builds | |
| npx eas build --profile development | |
| # For local builds | |
| npx eas build --profile development --local |
🤖 Prompt for AI Agents
In content/docs/react-native/installation.mdx around lines 69 to 75 the
documentation uses the incorrect EAS CLI invocation "npx eas-cli", which will
fail because the correct binary is "eas"; update both examples to use "npx eas"
(e.g. "npx eas build --profile development" and "npx eas build --profile
development --local") so the commands work as intended.
Description
Testing (ignore for documentation update)
Type of change
Checklist:
Additional Notes:
Summary by CodeRabbit