Closed farfromrefug closed 2 months ago
If your changes become huge, those won't be merged into the official repo. Please split them into branches, make pull requests and only then it's possible to review and merge. Also, depends on whether @itkach has any will to continue this project.
@soshial yes i know. But the migration has to be a big one. Androidx/material are really linked. And yes it depends on @itkach wanting to continue. @itkach if you need help maintaining this i am really interested in helping.
BTW, there have been several huge forks with full rework of this app (but I suppose those are stale now) — you may find cool things to rebase on or improve on.
@soshial Nice did not know. Will take a look
@farfromrefug thank you, I appreciate the effort. So far it looks mergeable. Even though it is technically quite big in terms of number of changes, most of these changes appear to be rather mechanical/cosmetic, nothing changes conceptually. I do see, however, some changes unrelated to androidx migration already crept in: for example replacing switch statements with ifs here and here and in other places, or an attempt at issue fix, or merging a pull request, or tooling updates. While such changes are not necessarily blockers (and some may even be unavoidable, like tooling updates aggressively forced by Google), they do make it more difficult to review and test, and may make it impossible to merge if enough of them are accumulated in one PR.
BTW, there have been several huge forks with full rework of this app (but I suppose those are stale now) — you may find cool things to rebase on or improve on.
If that's what you would like to do (full rework) then I think the best course of action would be to keep that as a separate project and release it under a new name. I'd be happy to recommend users check out such a new application when it is ready.
depends on whether @itkach has any will to continue this project.
I achieved my personal goals I had with this project a long time ago. I do keep using it myself (lightly) and I would like to keep it in working order for the foreseeable future if I can, but I do not plan or want any new major development. Small fixes and tweaks, occasionally catching up with Android tooling, refreshing the look/theme - sure. Re-architecting the app, re-thinking navigation patterns and UI in general, re-organizing the code, re-writing in Kotlin (as someone suggested once) - all these may be great ideas potentially resulting in great new app(s) but I think they are best pursued as their own separate projects.
@itkach switch migration to if is needed because it is not supported anymore in latest java. For the other changes indeed ( you did looked at it all ;)) i will move them to a separate PR. I am working on multiple things at the time. Dont worry i will separate all that!
As for the future and big updates like architecture, navigation,..., i am willing (if it is something i need as my personal need for this app) to implement them. If you agree you could add me as to this repo and i could implement such things with PRs so that you can still have a look at it
@itkach switch migration to if is needed because it is not supported anymore in latest java.
Hm... that would be a pretty incredible breaking change in Java, and indeed I can't find any documentation that would support your statement. Can you elaborate?
You can read more about problems with switch statements here.
Basically it won't compile if the integer is not final:
Ah, so switch is supported in latest Java just fine - phew, got me worried there for a moment! 😌 - this is android tooling issue, thank you for clarifying. Looks like, more specifically, it's a breaking change in Android Gradle Plugin 8.0.0 defaults. Since this defaults change doesn't benefit this app in any meaningful way, an easy way to avoid changing the switch statements would be to explicitly set android.nonFinalResIds
to false or even better - gasp! - to not make gradle plugin upgrade part of these changes.
android.nonFinalResIds=false
will probably help against this change. Also, I think we need to upgrade the gradle plugin, because it's needed to use the latest Android Studio. But I am not 100% sure about this. @farfromrefug
android.nonFinalResIds=true
You mean android.nonFinalResIds=false
. true
is the new default, false
is the previous default.
I am not sure ... will help against this.
Why not?
Updated my answer
@itkach i think upgrading to gradle 8 is kind of a must have. Anyway at one point you will be force too to be compatible with latest targets and thus release with new features. As for the change to switch to if, I dont think what the issue is. If it is the default in latest gradle/java I guess it means it is optimized in some way. Otherwise the default would have remain to allow the switch.
But anyway it is your choice, just dont see the issue in migrating to if
As for the change to switch to if, I dont think what the issue is. If it is the default in latest gradle/java I guess it means it is optimized in some way. Otherwise the default would have remain to allow the switch.
It optimizes something that this application doesn't need optimized while forcing unnecessary code changes. This is a misguided default change - which is par for the course for Android.
Anyway, this conversation started with you saying "switch migration to if is needed ..." and I'm just pointing out that's not the case, that's all.
@itkach yes sure dont worry I ll create à PR for what you accept and the rest will remain in my fork. Might just come a point where I cant PR anymore as it does not align with my goals. At that point I might create a new app. Thanks a lot for everything you Ve done.
I am still "comparing" your slob format and zim. Your slob are much smaller which interest me. Plus I cant get remote content load to work in zim/kiwix
BTW, @farfromrefug, if you maintain this app (or its fork), I'll come in and help from time to time with some features/bugfixes/improvements, because I use this app really often.
@soshial thanks a lot. Yes i think i might have to go for a work. I am already making more changes for better usability and i have more ideas.
BTW, since you're migrating to Material Theme, @farfromrefug, it would be cool to migrate the tabs as well:
@soshial Yes i plan on going full material. Just gonna take some time as this mean true rewrite not just theme / style migration
@soshial here you go the app is now using kotlin and using material components: I think i am past the post of simple PRs as @itkach mentioned. Guess it is time for a new app! @itkach thanks again for your feedback. If at any point you want us to "merge" and start working together on that "new" code base, please let me know. This is the route i would prefer
Wow, this looks pretty much amazing, @farfromrefug! 🔥 Thanks for removing that weird font file and making the app completely in Kotlin. So what are your plans for the app? How are you going to name it?
@soshial i have no idea :D Materiaard ? :P I am also working on making the article loading much faster. Until now it was quite slow because multiple articles were loaded at the same time
Just so it is known and to prevent someone starting this. I started working on upgrading the project and improving UX / features. The work is happening here https://github.com/farfromrefug/aard2-android Here are a few screenshots