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

Support Carthage installation #17

Merged
merged 6 commits into from Mar 7, 2018

Conversation

yuzushioh
Copy link
Contributor

@yuzushioh yuzushioh commented Mar 6, 2018

Issue/Task Number: Link

Overview

Supports SDK installation via Carthage.

Changes

  • Create Carthage build of sdk
  • Update README in installation section

Implementation Details

I make changes to support installation via Carthage.

Usage

  1. Create Cartfile in some directory and add github "omisego/ios-sdk", then run carthage update --platform ios
  2. This creates directory named Carthage and put the built framework in it.
  3. Add the OmiseGo framework to the project and run.

Impact

None

@mederic-p
Copy link
Contributor

Hi, thanks for contributing to the project!
It's nice to have a support for Carthage, however I don't think that we should include the built framework in the repo for now. as we're still in beta, we will be changing the code almost everyday and it doesn't really make sense.
Also I think that most of swift projects don't actually embed the built framework in the sources but instead let it compile when running carthage update --platform ios.
Can you please remove the Carthage folder for now and we will see if we need to add it later.
It'd be nice also to have the last step in the readme (drag the framework in the project).

@mederic-p mederic-p self-assigned this Mar 7, 2018
@yuzushioh
Copy link
Contributor Author

yuzushioh commented Mar 7, 2018

@mederic-p Thank you for the review.

I don't think that we should include the built framework in the repo for now. as we're still in beta, we will be changing the code almost everyday and it doesn't really make sense.

This makes total sense for me too. I removed the build framework from the repository.

most of swift projects don't actually embed the built framework in the sources

I meant to drag the framework to the project by embed the built framework. I changed it.

README.md Outdated
carthage update --platform ios
```

Drag `OmiseGo` in the project.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd put:
Drag the built OmiseGO.framework into your Xcode project.
For maximum clarity :)

Copy link
Contributor

@mederic-p mederic-p left a comment

Choose a reason for hiding this comment

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

Just a minor wording change

@yuzushioh
Copy link
Contributor Author

@mederic-p fixed 👍

@yuzushioh yuzushioh changed the title [Proposal] Support Carthage installation Support Carthage installation Mar 7, 2018
@T-Dnzt T-Dnzt removed their assignment Mar 7, 2018
@mederic-p mederic-p merged commit 145ad08 into omgnetwork:master Mar 7, 2018
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.

None yet

3 participants