Charts: Move Realm integration to another repository

Created on 31 Oct 2016  路  20Comments  路  Source: danielgindi/Charts

Have you considered moving ChartsRealm to another repository, that depends on both Charts and Realm?

Right now, they are both in this repository and in the same project. That causes some dependency managers, like Carthage, to build all Charts, ChartsRealm and Realm itself, even if its integration is not used.

discussion

Most helpful comment

+1 to removing the Realm dependency.

It isn't a core part of the Charts library or even related; it is a nice to have. You already did the work to split the dependencies, so separate repo seems to make sense. AlamoFire does the same thing.

For me, it isn't about build times or anything, just project maturity. Seeing Realm pop up unexpectedly in Cartfile.resolved threw a major red flag in my team.

All 20 comments

We have. A lot. Unfortunately this complicates things...
ChartsRealm needs Charts and Realm. ChartsDemo is obviously managed as part of the Charts repository for simplicity and accessibility.
As ChartsDemo provides Realm demos as well, it depends on Charts and ChartsRealm.

So this makes: Charts repo will depend on ChartsRealm repo which will depend on Charts repo.

Currently, both Pods and Carthage do not support any kind of cyclic dependencies. They did not implement even the simplest algorithm to find redundancies like this and resolve them, as in practice it can get complicated.

So for ease of use we are keeping things as it is right now.
If there will be more votes for having this separated along with its demos, we may consider this.

I'll leave this open for now.

I understand, thanks for the quick answer!

In the meantime, I think I'll create a quick fork and remove Realm dependencies in there. I hope this will resolve my 20-min-long carthage update 馃檲

Make sure to update your fork frequently, as important bugfixes and new features come in....

If you find Carthage takes too long use cocoapods. You can specify to only build charts and not chartsrealm since they have subspecs.

I tried out Carthage a while back but went back to cocoapods because it's faster and simpler.

That's subjective. For me, CocoaPods is easier, but Carthage is simpler. 馃槈

Ya I meant easier ;)

@danielgindi can you update realm to 2.x ?

Pull requests are always welcome 馃槈

@danielgindi Seems like the current ChartsDemo is really a ChartsRealmDemo. You could move this demo and Realm to a separate project, and provide a bare-bones ChartsDemo with no realm dependencies. Just add a note that a more full-featured demo is available in the ChartsRealm project.

Moving ChartsRealm out of Charts will save our dev team lots of pointless download and building time.

@getaaron, not all of it, ChartsDemo still holds more demos without realm. I agree we could separate them, but so far, I just needed one time building and downloading for Realm, by carthage. It should not build all the time?

@danielgindi I'm in favor to move ChartsRealm to another repo.
This take so long to build with carthage...

*** Building scheme "RealmSwift" in Realm.xcworkspace
*** Building scheme "Realm" in Realm.xcworkspace
*** Building scheme "PlaygroundFrameworkWrapper" in RealmExamples.xcworkspace
*** Building scheme "ChartsRealm" in Charts.xcodeproj
*** Building scheme "Charts" in Charts.xcodeproj

This make me crazy because I only use Charts...

Well Carthage really doesn't want to add subspecs so if you don't want to download everything all the time use cocoapods or complain to the Carthage maintainers. Separate repos make our lives much harder for little gain. We could split the demos into 2 targets but that really doesn't fix some of the problems mentioned here.

+1 to removing the Realm dependency.

It isn't a core part of the Charts library or even related; it is a nice to have. You already did the work to split the dependencies, so separate repo seems to make sense. AlamoFire does the same thing.

For me, it isn't about build times or anything, just project maturity. Seeing Realm pop up unexpectedly in Cartfile.resolved threw a major red flag in my team.

I agree with @donnellyk.

@donnellyk Until we think of a solution that answers the major concerns - we can't do that. And believe me, we want to!

@danielgindi Fair enough. Which concerns are that? Maybe we can help? 馃檪

At first we thought about keeping the Realm stuff in the project - for ease of maintainability.
The problem arising from it is cyclic dependencies not being supported by both CocoaPods and Carthage.

Now we're left with the desire to keep the Demo with the showcase of Realm related stuff, both for the users to be accessible, and for us when testing while developing.
Think about this: There's a bug in Charts which affects ChartsRealm, or a utility function that we're adding in favor of ChartsRealm. The development process would be to either:

  1. Do stuff on Charts clone, copy over to the Pods folder in ChartsRealm clone, see that it Builds, then run the Demo and test it visually, then maybe run some unit tests.
  2. Do the same on the Pods folder in ChartsRealm, and then copy over to Charts clone.

The original main concern was more about the users: There's very high demand to ChartsRealm. Current situation makes ChartsRealm very visible and accessible.
But now that we get many rejects from vanilla Charts users, who are not aware that Realm is just an optional dependency (or that it needs to be fetched for the Demo to work) - I'm thinking our concerns were upside-down.

About developing flow - Maybe we should just bite the bullet, get used to it. There's not much develpoment going on on the ChartsRealm side anyway (except for implementing the new ObjC support stuff on Realm which was released a few days ago following my PR).

I was generally very busy lately with many things - so I want to sit down and close some issues, merge PRs etc., and then maybe try to separate the ChartsRealm repo and see how it goes.

@danielgindi finally catch you here. Yes we have many things to fix before thinking about Realm.. 馃槀

Yeah it's been some busy days :-) Still are...
But I'm taking care of some issues and PRs, moving towards the next minor release,
and at that time we'll probably separate realm stuff.

Okay so I'm done splitting the repo and moving the realm stuff to https://github.com/danielgindi/ChartsRealm.
Travis is still broken, @petester42 could you take a look?

Was this page helpful?
0 / 5 - 0 ratings

Related issues

brytnvmg picture brytnvmg  路  4Comments

newbiebie picture newbiebie  路  3Comments

heumn picture heumn  路  3Comments

valeIT picture valeIT  路  3Comments

Aungbandlab picture Aungbandlab  路  4Comments