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

Extracting ChartsRealm to a separate project #1119

Closed
eshurakov opened this issue Jun 9, 2016 · 19 comments
Closed

Extracting ChartsRealm to a separate project #1119

eshurakov opened this issue Jun 9, 2016 · 19 comments

Comments

@eshurakov
Copy link

I think it would be nice to separate Charts and ChartsRealm into two projects.

There are several issues with having them in the same project:

  1. Building with carthage takes a lot of time as it builds both Charts and ChartsRealm. For me it also randomly fails when compiling ChartsRealm.
  2. Big repository size as Realm framework is checked in

If this is split into two projects, ChartsRealm could have a dependency on Charts (via carthage or as a submodule) and on Realm.

@liuxuan30
Copy link
Member

they are two projects on master, could check if I am wrong?

@eshurakov
Copy link
Author

Sorry, my bad, I meant separate repository.
The thing with carthage is that it builds every shared scheme in the repository, which now includes both Charts and ChartsRealm frameworks. Of course, this sounds like a problem that carthage should solve, I still feel like ChartsRealm deserves its own repository. But I can also see a problem that questions about charts will most likely be asked both here and there.

Regarding my initial issue with carthage, as a workaround I forked Charts repository and removed Realm so it doesn't take time to compile code that I don't use.

@ahknight
Copy link

Agreed. This is the best solution all around. One repo, one project. (Having similar issues with Carthage, myself.)

@liuxuan30
Copy link
Member

Putting ChartsRealm as a separate repo seems a overkill? It is just a small part of the Data series

@ahknight
Copy link

Perhaps, but including it confuses a major package manager and a good segment of the user base.

@liuxuan30
Copy link
Member

alright, guess we will need @danielgindi review this

@liuxuan30
Copy link
Member

also seems related to #1118 ?

@danielgindi
Copy link
Collaborator

danielgindi commented Aug 5, 2016

Truth is I really want to do this - so I won't have to include binaries of Realm in the project.
I don't mind keeping it in the project or moving to a separate repository - as long as regular users can compile the project (and run the demos too) right away without errors,
And Realm users can use it easily without a hassle.

(Keeping Carthage/Pods compatibility is nice too)

I'm open to suggestions here on how to do this!
If possible, this should be all set up with the v3 release.

@petester42 do you have any idea how to do this?

@pmairoldi
Copy link
Collaborator

I have ideas on how to make this better so binaries are not included. It won't address the first point in the issue though.

@danielgindi
Copy link
Collaborator

Well let's hear it :-)

@pmairoldi
Copy link
Collaborator

Basically what I would do is what I did for (this project)[https://github.com/Moya/Moya]. One project with all the targets. Carthage to download the dependencies for the project. Cocoapods shouldn't really change because of this.

@danielgindi
Copy link
Collaborator

@petester42 So you are suggesting to move the Realm to a separate project, and the demos as well?

I would have moved just the Realm stuff - but I don't think that Pods or Carthage would handle cyclic dependencies that well...

My other option is moving Realm, with the specific Realm demos...

@pmairoldi
Copy link
Collaborator

I tried to update the project structure but Carthage was having problems. What you could do to remove the binaries is add a cartfile in the project root with realm. Then reference the binaries from the Carthage build folder instead of the ones in the repo.

@danielgindi
Copy link
Collaborator

But then we need some kind of optional dependency on them, to allow compilation without. I don't want people to have to have Carthage installed.
I've heard that Swift 3 has a new canImport keyword? Maybe that would help...

@pmairoldi
Copy link
Collaborator

We could use gitsubmodules and reference the project directly. To me always ends in a world of pain but it is an option.

I believe that Cocoapods and Carthage are ingrained in the community enough for it to be acceptable to have it be required to build the project.

@danielgindi
Copy link
Collaborator

Yes git submodules isn't an option. People need to know how to check it out, and we need to be careful to update the references. They really need to come up with a better submodules solution.

I'm not using Cocoapods or Carthage myself. I hate pods for managing my project structure, and I hate carthage for many bugs I encountered along the way while compiling Charts...
I think that I'll use a package manager when it will be an embedded solution, supported by creators of the IDE...
Think about npm for node, NuGet for Visual Studio, Gradle for Android Studio, apt-get for Linux, _______ for Xcode...
Apple is working on it - https://github.com/apple/swift-package-manager

Anyway, I think I'll try to do this when Swift 3 is final - have an optional dependency in the demo, and if downloaded via Carthage - then the demo will show the Realm stuff.

@liuxuan30
Copy link
Member

lol I think no pods/carthage binding is good idea, though I think carthage sometimes works as expected.. I don't use pods too

@danielgindi
Copy link
Collaborator

Well, I've tried with canImport, but it seems to not be in the final Swift 3.0, despite the talk about it. The guys in CocoaPods seem to have decided against it too. (CocoaPods/CocoaPods#5631)

So for the meantime I had to commit prebuilt binaries for Realm - but I REALLY want to get ANY binaries out of git, as it is a bad idea.
So I'll go with @petester42's suggestion. The ball is in your court!

@pmairoldi
Copy link
Collaborator

Carthage building has been fixed to be reliable and we decided to not split Charts and ChartsRealm into 2 repos since maintaining the library would require more work.

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

No branches or pull requests

5 participants