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

Update dependencies and tools to target API 33 #680

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Zerwin
Copy link
Contributor

@Zerwin Zerwin commented Aug 25, 2024

Changes

Updates the dependencies, Gradle and build tools. Several linting warnings and errors are also fixed.

Checklist

  • My change builds
  • I tested my change
  • I use the "bttv_" prefix for all resources I propose
  • When adding a string I also added it to the bttv.Res.strings Enum and res/values/strings.xml (in mod) and res/values/public.xml (in disass)
  • If my change is significant enough, I added it to the CHANGELOG.md under master
  • I'll add myself and everyone else who contributed to this change to the contributors list using all-contributors
  • I license my changes according to the MIT License.

I did not run ubi, because the build tools 33 use d8 instead of dx now but ubi itself seems to have dx hardcoded in some places.

@Zerwin
Copy link
Contributor Author

Zerwin commented Aug 25, 2024

Seems like the build failed because the KSPASS variable was missing in the CI pipeline. Not really sure why though.

@Zerwin Zerwin force-pushed the update-dependencies-33 branch from fd15ec2 to b8d86d8 Compare August 25, 2024 19:55
Copy link
Member

@FoseFx FoseFx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to delay merging this until we have rebased to a more recent version, as these changes might cause tricky to debug issues, for which one reason may be an outdated base.

@@ -21,6 +21,14 @@ public EmoticonToken(String text, String id) {
this.id = id;
}

public final String component1() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dirty workspace?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TokenizerTest still uses these functions

System.out.println("name: " + ((MessageToken.EmoticonToken) token).component1());

Although in hindsight adjusting the test would probably be the better way forward.

mod/app/src/main/java/bttv/updater/UpdaterJobService.java Outdated Show resolved Hide resolved
mod/app/src/main/java/bttv/updater/UpdaterJobService.java Outdated Show resolved Hide resolved
@Zerwin
Copy link
Contributor Author

Zerwin commented Aug 26, 2024

I'd like to delay merging this until we have rebased to a more recent version, as these changes might cause tricky to debug issues, for which one reason may be an outdated base.

Sounds good to me, as it mostly touches files that aren't usually changed in rebases it shouldn't be a problem to do this later.

@Zerwin Zerwin force-pushed the update-dependencies-33 branch from 9d29975 to 1a6339f Compare September 2, 2024 22:32
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

Successfully merging this pull request may close these issues.

2 participants