search

loadsmart / rn-salesforce-chat

React Native wrapper for Salesforce SDK chat
MIT License
7 stars 10 forks source link

Chore/react CocoaPods migration #11

Closed lenoirzamboni closed 4 years ago

lenoirzamboni commented 4 years ago

Types of changes

Reason for the changes

When you create a library using the react-native create module command, the version of react-native is 0.41 by default. This version of react-native comes with the React.xcodeproj. We were importing the React.xcodeproj as a build dependency, to resolve our RN dependencies.

Since this version of the react-native is very old, it could lead to unexpected errors, that's why we decided to update the react-native version to a newer one.

When react-native was updated to 0.61.2 (in the previous PR #10), the React.xcodeproj was not more exported along with the react-native dependencies, meaning we were unable to build the library.

Knowing this, we could regress to a version <= 0.60, or we could try resolving the react dependencies using another way. And that's why we are willing to use CocoaPods to manage the react-native dependencies.

Description of the proposed changes

barbosa commented 4 years ago

@lenoirzamboni would you mind explaining a bit more the reasoning behind this PR?

barbosa commented 4 years ago

Maybe we should update PULL_REQUEST_TEMPLATE to make this:

Please describe your changes here to help reviewer, explaining the reason for this PR, and also you can include screenshots if your change is UI related. Please remember to include the type of change in your title (Bugfix, Chore, Feature, Refactoring).

... more explicit, by maybe breaking down these things (reasoning, screenshots, etc) into their specific sections, and not everything under Description of the proposed changes. What do you think, @lenoirzamboni ?

lenoirzamboni commented 4 years ago

@lenoirzamboni would you mind explaining a bit more the reasoning behind this PR?

@barbosa Sure. When you create a library using the react-native create module command, the version of react-native is 0.41 by default. This version of react-native comes with the React.xcodeproj. We were importing the React.xcodeproj as a build dependency, to resolve our RN dependencies.

Since this version of the react-native is very old, it could lead to unexpected errors, that's why I decided to update the react-native version to a newer one.

When react-native was updated to 0.61.2 (in the previous PR #10), the React.xcodeproj was not more exported along with the react-native dependencies, meaning we were unable to build the library.

Knowing this, we could regress to a version <= 0.60, or we could try resolving the react dependencies using another way. And that's why we are willing to use CocoaPods to manage the react-native dependencies.

lenoirzamboni commented 4 years ago

Maybe we should update PULL_REQUEST_TEMPLATE to make this:

Please describe your changes here to help reviewer, explaining the reason for this PR, and also you can include screenshots if your change is UI related. Please remember to include the type of change in your title (Bugfix, Chore, Feature, Refactoring).

... more explicit, by maybe breaking down these things (reasoning, screenshots, etc) into their specific sections, and not everything under Description of the proposed changes. What do you think, @lenoirzamboni ?

@barbosa Agree! Do you mind if I make this change on this PR? Also, which sections would you like PULL_REQUEST_TEMPLATE to have?

Something like that?

barbosa commented 4 years ago

@lenoirzamboni Sure. Go ahead 👍 These 4 look good 👌