Skip to content

minor typos and force compiler to use C++17 standard only for fmt package#20

Open
BogdanGeorgian91 wants to merge 1 commit intomicroblink:mainfrom
BogdanGeorgian91:main
Open

minor typos and force compiler to use C++17 standard only for fmt package#20
BogdanGeorgian91 wants to merge 1 commit intomicroblink:mainfrom
BogdanGeorgian91:main

Conversation

@BogdanGeorgian91
Copy link
Copy Markdown

@BogdanGeorgian91 BogdanGeorgian91 commented Apr 28, 2026

Was curios to give it a spin and found some minor typos and a compiler issue :)

Summary by CodeRabbit

  • Bug Fixes
    • Corrected package installation reference to ensure the proper dependency is installed from the registry
    • Applied build configuration patch to prevent compilation failures when using newer development tools and environments
    • Updated liveness configuration settings in card scanning handlers for improved hand-held card detection accuracy

standard only for fmt package
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 28, 2026

📝 Walkthrough

Walkthrough

The PR corrects the BlinkCard React Native package name from a misspelled variant, adds a Podfile build configuration patch for Clang C++ compatibility, and updates a liveness configuration property name across three scan handlers in the sample application.

Changes

Cohort / File(s) Summary
Build Script & Configuration
initBlinkCardReactNativeSample.sh
Fixed package name spelling from blinkcar-react-native to blinkcard-react-native. Added Podfile post-install patch to set CLANG_CXX_LANGUAGE_STANDARD to c++17 for the fmt pod to resolve Xcode 16 / Clang compatibility issues.
Application Configuration
sample_files/App.tsx
Updated liveness configuration in three scan handlers by renaming property enableCardHelpInHandCheck to enableCardHeldInHandCheck, maintaining all existing values set to true.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A package name once spelled with care,
Now corrected, beyond compare!
Build configs patched with C++ delight,
Properties renamed, everything right—
BlinkCard bounces with joy so bright! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main changes: fixing a typo in the package name and forcing C++17 for the fmt package compilation.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with 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.

❤️ Share

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

Copy link
Copy Markdown

@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: 1

🧹 Nitpick comments (1)
initBlinkCardReactNativeSample.sh (1)

66-68: Fail fast when Podfile patch is not applied.

Line 67 can silently no-op if the template changes. Add a post-check so the script exits immediately when the fmt C++17 override is missing.

Suggested hardening
 # Fix for fmt consteval error in Xcode 16 / newer Clang
 perl -i~ -pe "BEGIN{$/ = undef;} s/(react_native_post_install\([^)]+\))/\1\n\n    # Fix for fmt consteval error in Xcode 16 \/ newer Clang\n    installer.pods_project.targets.each do |target|\n      if target.name == 'fmt'\n        target.build_configurations.each do |config|\n          config.build_settings['CLANG_CXX_LANGUAGE_STANDARD'] = 'c++17'\n        end\n      end\n    end/s" Podfile
+grep -q "CLANG_CXX_LANGUAGE_STANDARD.*c++17" Podfile || {
+  echo "Podfile patch for fmt/C++17 was not applied"; exit 1;
+}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@initBlinkCardReactNativeSample.sh` around lines 66 - 68, After applying the
perl substitution that inserts the C++17 override inside the Podfile's
react_native_post_install block, add a post-check that reads the Podfile and
verifies the presence of the inserted block (e.g., that the Podfile now contains
the lines referencing target.name == 'fmt' and CLANG_CXX_LANGUAGE_STANDARD =
'c++17'); if the check fails, print an error and exit non‑zero so the script
fails fast. Locate the perl insertion around the react_native_post_install(...)
pattern and validate the presence of the 'fmt' target override immediately after
that substitution, exiting the script when the override is missing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@sample_files/App.tsx`:
- Line 88: The README example uses the wrong property name
enableCardHelpInHandCheck which will break copy/paste; update the README snippet
to use the correctly spelled property enableCardHeldInHandCheck (the same one
set in App.tsx) and verify any other examples or docs do not reference
enableCardHelpInHandCheck so examples and runtime code match exactly.

---

Nitpick comments:
In `@initBlinkCardReactNativeSample.sh`:
- Around line 66-68: After applying the perl substitution that inserts the C++17
override inside the Podfile's react_native_post_install block, add a post-check
that reads the Podfile and verifies the presence of the inserted block (e.g.,
that the Podfile now contains the lines referencing target.name == 'fmt' and
CLANG_CXX_LANGUAGE_STANDARD = 'c++17'); if the check fails, print an error and
exit non‑zero so the script fails fast. Locate the perl insertion around the
react_native_post_install(...) pattern and validate the presence of the 'fmt'
target override immediately after that substitution, exiting the script when the
override is missing.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 184a7216-1f52-4cf9-a1b5-3c6395b873d8

📥 Commits

Reviewing files that changed from the base of the PR and between a65d13b and a306837.

📒 Files selected for processing (2)
  • initBlinkCardReactNativeSample.sh
  • sample_files/App.tsx

Comment thread sample_files/App.tsx
*/
const livenessSettings = new LivenessSettings();
livenessSettings.enableCardHelpInHandCheck = true;
livenessSettings.enableCardHeldInHandCheck = true;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Update README example to the same property name to prevent copy/paste breakage.

The sample now correctly uses enableCardHeldInHandCheck, but the snippet in README.md (lines 1-50 in provided context) still uses enableCardHelpInHandCheck.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@sample_files/App.tsx` at line 88, The README example uses the wrong property
name enableCardHelpInHandCheck which will break copy/paste; update the README
snippet to use the correctly spelled property enableCardHeldInHandCheck (the
same one set in App.tsx) and verify any other examples or docs do not reference
enableCardHelpInHandCheck so examples and runtime code match exactly.

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