Skip to content
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

add addRadarSDKVerify option #367

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

add addRadarSDKVerify option #367

wants to merge 13 commits into from

Conversation

nickpatrick
Copy link
Contributor

@nickpatrick nickpatrick commented Nov 26, 2024

@nickpatrick nickpatrick marked this pull request as ready for review November 30, 2024 19:13
@@ -59,6 +59,7 @@ export const withRadarAndroid = (
<!-- for React Native -->
<domain-config cleartextTrafficPermitted="true">
<domain includeSubdomains="true">localhost</domain>
<domain includeSubdomains="true">10.0.2.2</domain>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

localhost on Android emulator


// Insert the pod declaration within the target block
const targetBlock = contents.substring(targetStartIndex, targetEndIndex);
const updatedTargetBlock = targetBlock.replace(/(target '(\w+)' do)/, `$1\n pod 'RadarSDK/Verify', '3.19.2-beta.9'\n pod 'CocoaAsyncSocket', :modular_headers => true\n pod 'HTTPParserC', :modular_headers => true`);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Telegraph sub-dependencies need modular_headers => true

const contents = await fs.readFile(filePath, 'utf-8');

// Check if the pod declaration already exists
if (contents.indexOf("pod 'RadarSDK/Verify', '3.19.2-beta.9'") === -1) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lmeier @KennyHuRadar Similar to addRadarSDKMotion option. Will we need to update the version by hand in both places?

Copy link
Contributor

Choose a reason for hiding this comment

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

we should probably extract this to a constant in the future. Since we release all the sdks with matching versions (I think, since RadarSDKMotion releases a 3.19 together with a 3.19 main SDK even if there is no change). We can probably just update the version string in the podspec file, and we can read the version from there and use it across the plugin.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yea that's exactly correct — easiest to maintain the same version across the the main module and all submodules.

@KennyHuRadar
Copy link
Contributor

I noticed that you bumped the RN version, however, there are a few more spots where we need to bump the beta version and the native versions as well. It's documented here

  • we need to bump the rn version in the build.gradle
  • we need to define the rn version in android/src/main/java/io/radar/react/RNRadarModule.java and ios/RNRadar.m
  • we also need to bump ios native version in the cartfile

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.

4 participants