markusfisch / ShaderEditor

Android app to create GLSL shaders and use them as live wallpaper
https://play.google.com/store/apps/details?id=de.markusfisch.android.shadereditor
MIT License
924 stars 137 forks source link

AndroidX migration #147

Closed AntonPieper closed 1 year ago

AntonPieper commented 1 year ago

This migrates the codebase to AndroidX, because com.android.support:appcompat-v7 is deprecated

I also raised the minSdkVersion to 14 so that the project compiles. I think this is reasonable, because Android 4.0 (SDK Version 14, released 2011) is so far back in time that I don't think anyone has a phone with an older android version (and wants to run ShaderEditor on that).

Edit: Maybe The compile error could have been caused by my gradle.properties, I had set android.enableJetifier=true. Either way, I think that raising the minSdkVersion is better, because the <uses-sdk>-approach seem kind of hacky.

markusfisch commented 1 year ago

First of all thanks a lot for breaking things up into individual PRs! 👍

Just one tiny little thing: could you please add a gradle.properties files with the following content to make this commit compile?

android.enableJetifier=true
android.useAndroidX=true

Of course, I could add it in a subsequent commit. But I think it would be better if this commit would compile 😉

AntonPieper commented 1 year ago

Yeah, the android.useAndroidX=true setting is required. I want to note that this would require the file to be removed from the .gitignore. With the other PRs still open, I think it makes more sense to merge/reject them before this one as it's less work to convert them all at once instead of one by one (would need to merge/rebate the changes into each branch).

markusfisch commented 1 year ago

Yes, gradle.properties would have to be removed from .gitignore now, but you could also use the -f flag to force add it, if that's easier for now:

$ git add -f gradle.properties

Maybe I'm wrong, but I think you can add this to this branch (and push it into here) without needing to touch the other PRs 🤔 Could you please have a try? I cannot add something to this PR, unfortunately. And please excuse me for being so picky 🙈 I really appreciate your work! I just maybe care a bit too much about the commit history, and we're so close to the perfect commit 😉

PS: If the new commit would cause problems for the other PRs, we still can take it back and force push the new HEAD in this branch.

AntonPieper commented 1 year ago

I have added the gradle.properties file to the repository now. You are actually correct, the other PRs are unaffected (except #149, because it extends AppCompatEditText here)

markusfisch commented 1 year ago

Thanks a lot, and thanks again for adding the gradle.properties 👍

AntonPieper commented 1 year ago

I have just noticed that the Android code style separates the import android.* and import androidx.* imports, should this behaviour be enforced, or should I edit the .editorconfig file to not put blank lines in between? With the blank lines, reformatting changes basically all files.

markusfisch commented 1 year ago

No, that's okay, I've got no hard feelings about how the imports are formatted as long as it's consistent.

I would just want to avoid double blank lines.