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

Rename appcenter.podspec to appcenter-core to avoid conflicts #789

Merged
merged 8 commits into from
Jan 8, 2020

Conversation

russelarms
Copy link
Contributor

@russelarms russelarms commented Dec 20, 2019

Things to consider before you submit the PR:

  • Has CHANGELOG.md been updated?
  • Are tests passing locally?
  • Are the files formatted correctly?
  • Did you test your change with either the sample apps that are included in the repository or with a blank app that uses your change?

Description

Rename appcenter.podspec to appcenter-core to avoid conflicts.

Repro of the bug:
react-native init newapp && cd newapp
npm install appcenter appcenter-analytics appcenter-crashes --save-exact
cd ios && pod install && cd ..
rios
cd ios && rm -rf Pods && rm Podfile.lock && pod instal

Related PRs or issues

AB#71995
Github issue

@russelarms russelarms marked this pull request as ready for review December 20, 2019 16:56
@russelarms
Copy link
Contributor Author

I've tested it with the TestApp and with a new sample app. In both cases, it worked fine.

@russelarms
Copy link
Contributor Author

We have to test it with RN < 0.60, as for now, I have some issues with it. Maybe they are local.

Ruslan Urmeev and others added 2 commits December 23, 2019 13:30
@russelarms
Copy link
Contributor Author

I've tested it on a sample app built with RN 0.59.9, works fine. So it's ready to be merged now.

@dhei dhei changed the title Rename appcenter.podspec to appcenter-rn to avoid conflicts Rename appcenter.podspec to appcenter-core to avoid conflicts Dec 24, 2019
@Jarred-Sumner
Copy link

excited for this to merge

ilyashumihin
ilyashumihin previously approved these changes Dec 30, 2019
MatkovIvan
MatkovIvan previously approved these changes Dec 30, 2019
Jamminroot
Jamminroot previously approved these changes Dec 30, 2019
CHANGELOG.md Show resolved Hide resolved
@jaeklim
Copy link
Contributor

jaeklim commented Jan 6, 2020

This PR contains two items. Let's avoid combining multiple items in a PR.

CHANGELOG.md Outdated Show resolved Hide resolved
@annakocheshkova
Copy link

annakocheshkova commented Jan 7, 2020

@jaelim these two fixes only work in a bundle. One is not enough without the other.

Copy link
Member

@guperrot guperrot left a comment

Choose a reason for hiding this comment

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

Otherwise looks good to me.

@jaeklim
Copy link
Contributor

jaeklim commented Jan 7, 2020

@annakocheshkova Well, it seems to me like one name change and then one static framework change on top of name change. Am I wrong?

@guperrot
Copy link
Member

guperrot commented Jan 7, 2020

According to me it would be counter productive to split PRs at this point but let's keep that in mind for future PRs. All files have been already reviewed anyway.

Co-Authored-By: Di Hei <[email protected]>
@russelarms
Copy link
Contributor Author

russelarms commented Jan 8, 2020

  • applied changelog fix
  • added a comment inside the podspec file
  • tested the change with both cocoapods v.1.7.5 and v1.8.4

@russelarms russelarms requested a review from guperrot January 8, 2020 09:50
@guperrot guperrot merged commit 048a5f6 into develop Jan 8, 2020
@guperrot guperrot deleted the fix/rename-appcenter-podspec branch January 8, 2020 20:05
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.

9 participants