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

Migrating from string resources to SPI #503

Closed
zTrap opened this issue May 4, 2020 · 16 comments
Closed

Migrating from string resources to SPI #503

zTrap opened this issue May 4, 2020 · 16 comments

Comments

@zTrap
Copy link
Contributor

zTrap commented May 4, 2020

What do you think about migrating to Java Service Provider Interface?

@mikepenz
Copy link
Owner

mikepenz commented May 4, 2020

If we can make it work for android without any major overhead I would say absolutely.
One thing which is very relevant all the time though... obfuscation :D

@zTrap
Copy link
Contributor Author

zTrap commented May 4, 2020

We can make this, but we need to switch to jar packaging for typeface libraries. And obfuscation work perfectly with SPI :)

@mikepenz
Copy link
Owner

mikepenz commented May 4, 2020

Why would we need the jar packaging? the aar packaging is like a jar with additional features.
Maybe I miss something important here?

@zTrap
Copy link
Contributor Author

zTrap commented May 4, 2020

Aar removes META-INF folder from package, but SPI requires it

@mikepenz
Copy link
Owner

mikepenz commented May 4, 2020

hmmmm then there may be a different issue, as this may get lost in the end again via proguard. many people have a packaging rule to exclude meta inf files. due to conflicts of other libs.. :/

@zTrap
Copy link
Contributor Author

zTrap commented May 4, 2020

Hmm, ok, I’ll investigate this

@zTrap
Copy link
Contributor Author

zTrap commented May 4, 2020

My bad, it works perfectly with aar and auto-service (which adds all required information in META-INF automatically)

@mikepenz
Copy link
Owner

mikepenz commented May 4, 2020

Hmm I would propose that we have a look on how this meta data has to look like and we add it manually then to not rely on external dependencies.
but that sounds like it could work

@zTrap
Copy link
Contributor Author

zTrap commented May 4, 2020

I'we tried to add META-INF manually in res folder, but it wont work. Maybe I did something wrong... But I don't think this dependency is redundant. It's can be used in kapt configuration for processor and compileOnly for annotation, so nothing redundant is added to libraries

@mikepenz
Copy link
Owner

mikepenz commented May 4, 2020

but it will have to be mentioned in the licenses. and as it is a compile time dependency only, there is no reference left in the dependency tree, so AboutLibraries won't see it :D

Oh the META-INF won't be in the res folder. as it is "special" compile time.

I think we may be able to write to it via gradle :)

@zTrap
Copy link
Contributor Author

zTrap commented May 4, 2020

Sounds good :)

@mikepenz
Copy link
Owner

mikepenz commented May 4, 2020

Theoretically we could just use the ContentProvider of android to auto register the font dependencies...
https://medium.com/@andretietz/auto-initialize-your-android-library-2349daf06920

I am not really sure about the real world performance impact of having many contentProviders in an app :/

so let's say there are 5 fonts in an app. what would be the impact?

@zTrap
Copy link
Contributor Author

zTrap commented May 4, 2020

Idk, we need to test it

@mikepenz
Copy link
Owner

Well google just made this one a lot easier :D https://developer.android.com/topic/libraries/app-startup

@zTrap
Copy link
Contributor Author

zTrap commented Jun 11, 2020

Look's so pretty and easy. We'll wait for stable version?

@mikepenz
Copy link
Owner

I'd say we prepare a beta release of Android-Iconics v6 :D and wait for it's release until the app startup library is at least in beta :)

I'm gonna spend a bit of time on this, this week to prepare things :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants