Skip to content

feat: added possibility to start module with C++ code enabled #30

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
May 6, 2020
Merged

feat: added possibility to start module with C++ code enabled #30

merged 4 commits into from
May 6, 2020

Conversation

@Patys
Copy link
Contributor

@Patys Patys commented Mar 23, 2020 •
edited
Loading

Summary

  • Script ask you do you want to add C++ code to your module (by default set to "No")
  • It adds C++ files and additional code to load it to RN module
  • you can use code inside cpp folder to develop your module

Test Plan

  1. Run script
  2. Answer last question with Yes (Do you want to use C++ code?)
  3. Example contains multiply code

Compatibility

OS Implemented
iOS ✅
Android ✅

Checklist

  • I have tested this on a device and a simulator
  • I added the documentation in README.md
  • I mentioned this change in CHANGELOG.md
  • I updated the typed files (TS and Flow)
  • I added a sample use of the API in the example project (example/App.js)

satya164
satya164 approved these changes May 6, 2020
View reviewed changes
Copy link
Member

@satya164 satya164 left a comment

Choose a reason for hiding this comment

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

Thanks so much for the PR

@satya164 satya164 changed the title Added possiblity to start module with C++ code enabled feat: added possibility to start module with C++ code enabled May 6, 2020
@satya164 satya164 merged commit 6588c5c into callstack:master May 6, 2020
gengjiawen
gengjiawen reviewed Oct 29, 2020
View reviewed changes
templates/cppLibrary/android/CMakeLists.txt
set (CMAKE_VERBOSE_MAKEFILE ON)
set (CMAKE_CXX_STANDARD 11)

add_library(cpp
Copy link

@gengjiawen gengjiawen Oct 29, 2020

Choose a reason for hiding this comment

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

Maybe choose another name for library. cpp maybe a little confusing.

Copy link
Contributor Author

@Patys Patys Nov 7, 2020

Choose a reason for hiding this comment

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

Actually it's a good idea. I will try to address that in next pr 👍

gengjiawen
gengjiawen reviewed Oct 29, 2020
View reviewed changes
templates/cppLibrary/android/CMakeLists.txt
cmake_minimum_required(VERSION 3.4.1)

set (CMAKE_VERBOSE_MAKEFILE ON)
set (CMAKE_CXX_STANDARD 11)
Copy link

@gengjiawen gengjiawen Oct 29, 2020

Choose a reason for hiding this comment

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

Maybe 14 ? 11 is bit of old.

Copy link
Contributor Author

@Patys Patys Nov 7, 2020

Choose a reason for hiding this comment

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

I had issues with newer versions as it's compiled for ios and android. I will try to investigate it more in future to bump it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@satya164 satya164 satya164 approved these changes

+1 more reviewer

@gengjiawen gengjiawen gengjiawen left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Patys @satya164 @gengjiawen