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

Allow using cocoapods without react native link #244

Merged
merged 1 commit into from Apr 6, 2018
Merged

Allow using cocoapods without react native link #244

merged 1 commit into from Apr 6, 2018

Conversation

ghost
Copy link

@ghost ghost commented Feb 19, 2018

This helps existing projects which consume dependencies via cocoa pods and not project references.

I did not change the behavior or react-native link since I'm ok if this is just for people who feel confident to integrate manually.

I'm really stuck on using appcenter since I can't yarn install from git subdirectories, otherwise, I would consume my private branch. Please advise. I'm more than happy to take on a different path.

here's the discussion: #239

@msftclas
Copy link

msftclas commented Feb 19, 2018

CLA assistant check
All CLA requirements met.

@elamalani
Copy link
Contributor

@hccoelho Thanks for submitting a PR. Please give us a few days to review it and we will get back to you with our comments.

@guperrot guperrot changed the title Fix the podspecs and references so everything can go through cocoapods Allow using cocoapods without react native link Feb 23, 2018
@ghost
Copy link
Author

ghost commented Mar 4, 2018

Hi @guperrot, do you guys have thoughts on this?

@elamalani
Copy link
Contributor

@hccoelho Thanks for waiting to hear back from us. We have reviewed the PR and it looks like a good feature add to App Center SDK. However, we are not looking to add this functionality in App Center at this point of time.

My suggestion is to keep the PR open and we will get back to you once we decide to support it in App Center. Then we can work in collaboration to give you more direct feedback before merging in it our repo. Does that sound good?

@ghost
Copy link
Author

ghost commented Mar 5, 2018

Thank you for the update. Unfortunately, this is a blocker for me as I explain here: #239. Any thoughts on how to get me unblocked?

@dhei
Copy link
Member

dhei commented Mar 5, 2018

Hi @hccoelho, thanks for the PR.

Looks like you're blocked by not able to install git sub-directory with yarn to consume your own branch. Have you tried yarn add file:/path/to/local/folder with yarn (yarn documentation), or npm install /path/to/local/folder with npm (npm documentation)?

@ghost
Copy link
Author

ghost commented Mar 28, 2018

Thanks @dhei. The solution requires me to keep appcenter code in my repo which is not ideal. I would like to migrate from HA but with this limitation is quite hard. Any ETA on when you guys can accept this PR or we can iterate over it?

s.source_files = "appcenter-analytics/ios/AppCenterReactNativeAnalytics/*.{h,m}"
s.author = { 'Microsoft' => 'appcentersdk@microsoft.com' }
s.source = { :git => "https://github.com/Microsoft/AppCenter-SDK-React-Native.git" }
s.source_files = "AppCenterReactNativeAnalytics/**/*.{h,m}"
Copy link
Member

Choose a reason for hiding this comment

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

Was that required that you replace some part of the paths with stars?

Copy link
Author

Choose a reason for hiding this comment

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

Not required, however, this way it included sub-folders as well.

s.source = { :git => "https://github.com/Microsoft/AppCenter-SDK-React-Native.git" }
s.source_files = "appcenter-crashes/ios/AppCenterReactNativeCrashes/*.{h,m}"
s.source = { :git => "https://github.com/Microsoft/AppCenter-SDK-React-Native.git" }
s.source_files = "AppCenterReactNativeCrashes/**/*.{h,m}"
Copy link
Member

@guperrot guperrot Apr 3, 2018

Choose a reason for hiding this comment

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

I tested your branch on a sample project and this seems to work for cocopods project too. However before I merge, can you elaborate why it works to remove the beginning of the path, and what about the stars?

Copy link
Author

Choose a reason for hiding this comment

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

Yep: the source_files path is related to the root of the .podspec file. So we want to include in the podspec all .h and .m files (including sub-folders thus **/*.{h.m}) in AppCenterReactNativeCrashes folder starting from the root.

Starting from 2 levels up in the filesystem path would break the spec since source files would not be found.

@guperrot
Copy link
Member

guperrot commented Apr 3, 2018

@hccoelho just 2 comments above in the thread to clarify but otherwise it looks good. Will wait for your replies before merging.

@guperrot guperrot merged commit d61aa7d into microsoft:develop Apr 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants