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

customObject implementation #182

Merged
merged 8 commits into from
Feb 23, 2024
Merged

customObject implementation #182

merged 8 commits into from
Feb 23, 2024

Conversation

kober32
Copy link
Member

@kober32 kober32 commented Feb 16, 2024

Resolves #181

  • bumped iOS support to 13.4 as that's the current support of the newest react-native version.

This change should be released as 2.5.0

react-native-powerauth-mobile-sdk-2.5.0.tgz


project 'PowerAuth'

target 'PowerAuth' do
config = use_native_modules!
use_react_native!(
:path => config[:reactNativePath]
:path => config[:reactNativePath],
:hermes_enabled => false
Copy link
Member Author

Choose a reason for hiding this comment

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

Just comment here: this causes problem with the latest release and our integration and is only for our internal purposes, so disabled.

Copy link
Member

Choose a reason for hiding this comment

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

Could you describe the source of the problems? I was using "testapp" project only, for the development workflows. If I remember this correctly, then opening the standalone project was pain in the ass in recent RN stack, for both platforms. Using the "testapp" project was much better.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hermes is a Facebook-made javascript engine tailored for react native and brings faster startup times.

Basically, for a final build (when compiled), it improves performance but also brings many dependencies that need to be maintained and fixed when broken.

Since we use this only for our test projects and test apps, it's not needed.

Test projects for iOS and the test app still work but with the default js engine, not the Hermes.

@kober32 kober32 marked this pull request as ready for review February 20, 2024 12:26
@kober32 kober32 requested a review from hvge February 20, 2024 12:26
hvge
hvge previously requested changes Feb 22, 2024
Copy link
Member

@hvge hvge left a comment

Choose a reason for hiding this comment

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

The code looks OK. Please add docs/Version-2.5.md that describe possible migration instructions, even if there's none.


project 'PowerAuth'

target 'PowerAuth' do
config = use_native_modules!
use_react_native!(
:path => config[:reactNativePath]
:path => config[:reactNativePath],
:hermes_enabled => false
Copy link
Member

Choose a reason for hiding this comment

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

Could you describe the source of the problems? I was using "testapp" project only, for the development workflows. If I remember this correctly, then opening the standalone project was pain in the ass in recent RN stack, for both platforms. Using the "testapp" project was much better.

@kober32 kober32 requested a review from hvge February 22, 2024 13:52
Copy link
Member

@petrdvorak petrdvorak left a comment

Choose a reason for hiding this comment

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

Looks OK

@kober32 kober32 dismissed hvge’s stale review February 23, 2024 08:47

The issue was addressed and confirmed by the 2nd reviewer and the customer is waiting for the release.

@kober32 kober32 merged commit af34a2f into develop Feb 23, 2024
3 checks passed
@kober32 kober32 deleted the issues/181-customObject branch February 23, 2024 08:49
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.

Support customObject in PowerAuthActivationStatus
3 participants