search

readium / r2-testapp-swift

BSD 3-Clause "New" or "Revised" License
146 stars 38 forks source link

Improvements for clarity #211

Closed danielsaidi closed 5 years ago

danielsaidi commented 5 years ago

Hi,

When I work with your test app, I notice that you should probably remove Carthage dependencies from Cartfile, if they are implicitly resolved from other Carthage dependencies. It would make the test app more understandable, as you would understand which dependencies that are required for Readium and which are just added as app candy, e.g. the Hud dependency.

I understand that some of the app-specific dependencies are there, since this is an App Store published app, but perhaps it would be a good idea to have a super-slimmed test app, that only have the libraries you need to work with Readium, that also extract more logic from the various view controllers into understandable, smaller components?

mickael-menu-mantano commented 5 years ago

Hello,

The sub-dependencies are added to r2-testapp's Cartfile to be able to build the app when Readium 2's dependencies are integrated using submodules instead of Carthage. Granted this is not very clear right now, I will separate the sub-dependencies that are required only to work with submodules in the Cartfile.

For the test app we are actually aiming at having a simple app with few if any app-specific dependencies. SQLite.swift will probably stay, but we could do without Kingfisher and MBProgressHUD.