Conversation
This reverts commit 1db1a00. The issue has been long resolved making this fix no longer required.
Ref: https://developer.android.com/build/migrate-to-kotlin-dsl Signed-off-by: Aayush Gupta <[email protected]>
|
I have re-written the script to sort TOML file in Kotlin DSL as well while cherry-picking it from the refactor branch. |
litetex
left a comment
litetex
left a comment
There was a problem hiding this comment.
as per #10927 (comment)
We should do an independent update to the lastest Gradle version first and then have a look if it's useful to migrate everything to Kotlin
I am not sure what that comment means in relation to latest Gradle version first and Kotlin migration. I picked the TOML change as it has been merged for a long time in the refactor branch and is the current default for new android projects similar to how Kotlin is the new default DSL and language for writing new projects which we want to have with compose migration. Do you want me to bump the Gradle version to 9.1.0 in this PR as well? |
|
I was expecting that the change related to
Well you can do that but it should be done independently so that things don't get mixed up here :)
I see... I was not around when this was merged. I think I will create a PR to revert this on the refactor branch.
Even though Google might have declared this as the "default" it might not be very useful here. |
I feel it is useful considering we do seem to want a compose multi-platform compatible NewPipe which will involve us introducing multiple modules per-platform, sooner or later, leading us to require a way to manage multiple dependencies and plugins. Then there is also the logic of not following the changes done by Google because we don't like them leading us to have technical burden in future like today we have with Kotlin and Compose. |
|
@litetex it's actually the opposite, now all versions are in a single file instead of spread over different sections of |
Stypox
left a comment
Stypox
left a comment
There was a problem hiding this comment.
Thank you!
How do you know the reproducible builds fix is not needed anymore?
app/build.gradle.kts
Outdated
| // WORKAROUND: if you get errors with the NewPipeExtractor dependency, replace `v0.24.3` with | ||
| // the corresponding commit hash, since JitPack sometimes deletes artifacts. | ||
| // If there’s already a git hash, just add more of it to the end (or remove a letter) | ||
| // to cause jitpack to regenerate the artifact. |
There was a problem hiding this comment.
This should reference libs.versions.toml
There was a problem hiding this comment.
Removed it as the comment mentions to replace version with hash but it has already been using hash.
There was a problem hiding this comment.
Actually we keep switching back and forth, because when a new NewPipeExtractor version is released its tag works, but then at some point we need to use a development version of NPE or Jitpack stops providing artifacts for the tag, and so we have to switch to hashes.
As per the linked issue in the original commit, the fix has been shipped in AGP 8.1.0-alpha03 and we are using way newer version of AGP now. |
Ref: https://developer.android.com/build/migrate-to-catalogs Signed-off-by: Aayush Gupta <[email protected]>
Signed-off-by: Aayush Gupta <[email protected]>
Signed-off-by: Aayush Gupta <[email protected]>
theimpulson
force-pushed
the
buildImprovements
branch
from
05b0fd4 to
1bef2fd
Compare
I can't quite follow on this. Could you clarify what you mean with that? |
Specifying JDK toolchain in the java block lets us avoid specifying same version again and again for different options while ensuring everything is on the same version Ref: https://developer.android.com/build/jdks#toolchain Signed-off-by: Aayush Gupta <[email protected]>
|
Merging for now, despite the discussion in #10927 (comment) still not being concluded, in order not to block @theimpulson's other PRs. Since version catalogs have already been merged to refactor and the work has already been done, it wouldn't make sense to delay it further. We can always revert it later if more team members think we are better off without version catalogs, and the effort for reverting won't change if this PR is merged or not, since we'd have to revert it on |
The comments were accidentally removed in TeamNewPipe#12706
| /** Third-party libraries **/ | ||
| implementation(libs.livefront.bridge) |
There was a problem hiding this comment.
You missed this comment :-)
| /** Third-party libraries **/ | |
| implementation(libs.livefront.bridge) | |
| /** Third-party libraries **/ | |
| // Instance state boilerplate elimination | |
| implementation(libs.livefront.bridge) |
4 participants
What is it?
Description of the changes in your PR
Fixes the following issue(s)
APK testing
The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR. You can find more info and a video demonstration on this wiki page.
Due diligence