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

Remove globally available dependencies from project #5947

Closed
Pixelfahnder opened this issue Sep 10, 2016 · 14 comments
Closed

Remove globally available dependencies from project #5947

Pixelfahnder opened this issue Sep 10, 2016 · 14 comments

Comments

@Pixelfahnder
Copy link

I wonder if globally available dependencies like the android-support-v7-appcompat library should be removed from the source as they are available during the build process.

I see commits there but don't fully understand why they are done. As part of my starting UI overhaul I want to remove as much complexity as it could. So maybe I miss something here why this is needed.

Can someone clarify why we need the appcompat lib as part of the project but can't just simply rely on the dependency from Google.

@Pixelfahnder
Copy link
Author

The reason seems to be "include appcompat in repo to ease the build" why does it do so? The problem is, that also this library has been patched by Google and was actively developed by Google so I also guess this version may be completely outdated and not bug patched.

@Pixelfahnder
Copy link
Author

Also this may fix #4903 and #5602

@Pixelfahnder
Copy link
Author

I also discovered that the main project uses "compile 'com.android.support:appcompat-v7:20.0.0'" so it seems not to rely on the module in the repo.

@rsudev
Copy link
Contributor

rsudev commented Sep 10, 2016

The appcompat is in the repo for the sake of the Eclipse users.
As you just discovered, it has no bearing on the actual software or on anybody working with IntelliJ or AS.

@Pixelfahnder
Copy link
Author

@rsudev when is support for eclipse removed? cgeo supports so many IDEs / build system combinations; wouldn't it make sense to lower this down to have a cleaner codebase?

@rsudev
Copy link
Contributor

rsudev commented Sep 10, 2016

Likely not before I and @Bananeweizen stop contributing.
I seriously tried to switch to AS or IntelliJ for more than half a year. My productivity considerably raised again after switching back to Eclipse...

@Pixelfahnder
Copy link
Author

OK, then stop contributing :) Understandable; good point. Anyhow I had the same problems in the first hand but I more or less switched to IntelliJ completely for Android as well as for other Java / Python / ... stuff. I got completely used to it even if I suffer some problems.

I just wanted to remove code that is there from official sources to prevent beeing tied to old code. Have you tried to use gradle with Eclipse? If yes the appcompat could be used with the same repos as in IntelliJ. How do you build in Eclipse right now?

Either way, maybe we can update the appcompat lib that is checked in the repo as soon as we upgraded the dependency in the gradle files as well and fixed the maybe occuring code issues. I'm on that right now to test it.

@rsudev
Copy link
Contributor

rsudev commented Sep 10, 2016

Of course can the AppCompat in the repo be updated once we can switch to a newer version. Unfortunately does this have some not so nice side-effects due to our usage of GoogleMaps v1/Mapsforge 0.3. If you want to get an impression of what such a switch needs to address you can take a look at the branch https://github.com/cgeo/cgeo/tree/target_sdk_23. That worked basically, but some style issues, especially for the light skin, were still present.

@Pixelfahnder
Copy link
Author

@rsudev Styling should not be the problem as I plan to lift the design to a modern look and feel that can addres that maybe. What are the problems with the GMaps and Mapsforge APIs?

@rsudev
Copy link
Contributor

rsudev commented Sep 10, 2016

That they require the implementing activity to derive from their MapActivity (an IMO very stupid semantic introduced by GMaps v1 and mf was initially modeled after that).
This means, you cannot derive from the current ActionBar/AppCompat-Activity classes, thus have no ActionBar support for older Android versions and therefore need to fall back to the old implementation we still provide for older versions (currently < Api 13 IIRC). It works and looks bad and is not the best of ideas to force already on Lollipop users and below (we have only ~20% on MM right now)

@Bananeweizen
Copy link
Member

There is absolutely no relation between sources in the repository and the version of dependencies (in fact, the library sources and the gradle versions are not even identical). I update dependency versions of libraries and gradle plugins roughly every 2 weeks, where possible. Just check the git log of the main gradle file.

@Pixelfahnder If you can fix the mentioned incompatibilities between mapsforge and appcompat, you can completely ignore every library in the repo. I will easily fix that afterwards for the Eclipse users. It's just that we did not manage to fix this yet.

@Pixelfahnder
Copy link
Author

Pixelfahnder commented Sep 13, 2016

OK @Bananeweizen I thought it was only an issue with the GMAP thing but it also exist with the mapsforge APIs?

@rsudev
Copy link
Contributor

rsudev commented Sep 15, 2016

Both APIs (mapsforge v3 and google maps v1) have the same implementation semantics, where you have to derive from the respective MapActivity in order to use them. Therefore is our central map activity (CGeoMap) not even an Android activity at all.

@fm-sys
Copy link
Member

fm-sys commented Aug 30, 2021

android-support-v7-appcompat library should be removed from the source

A copy of the library is no longer contained in our repo

@fm-sys fm-sys closed this as completed Aug 30, 2021
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

4 participants