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

[flutter_local_notifications] Update gradle versions and declarations for example app #2482

Merged
merged 5 commits into from
Dec 14, 2024

Conversation

martin-bertele
Copy link
Contributor

@Levi-Lesches
Copy link
Contributor

Hey @martin-bertele, Maiku ran the CI just now, seems you're getting Java mismatches:

ERROR: ERROR: FAILURE: Build failed with an exception.

* Where:
Build file '/home/runner/work/flutter_local_notifications/flutter_local_notifications/flutter_local_notifications/example/android/app/build.gradle' line: 2

* What went wrong:
An exception occurred applying plugin request [id: 'com.android.application']
> Failed to apply plugin 'com.android.internal.application'.
   > Android Gradle plugin requires Java 17 to run. You are currently using Java 11.
      Your current JDK is located in /usr/lib/jvm/temurin-11-jdk-amd[64](https://github.com/MaikuB/flutter_local_notifications/actions/runs/12137271322/job/34172105150?pr=2482#step:5:65)
      You can try some of the following options:
       - changing the IDE settings.
       - changing the JAVA_HOME environment variable.
       - changing `org.gradle.java.home` in `gradle.properties`.

Notice how the Integrations tests are the only ones that passed on Android -- that's because they properly set up Java:

      - uses: actions/setup-java@v3
        with:
          distribution: 'zulu'
          java-version: '17'

Can you copy and paste that into the other failing tests as well?

@MaikuB
Copy link
Owner

MaikuB commented Dec 10, 2024

@martin-bertele I believe the build is failing as it needs changes that I tried to do in #2488 but looks like that would also need the changes done in this PR. Are you able to incorporate the changes I tried to do in this PR? Would help to ensure everything is green

Edit: missed that @Levi-Lesches made similar comments

@Levi-Lesches
Copy link
Contributor

Levi-Lesches commented Dec 12, 2024

@MaikuB If you can change your #2488 to be based on Martin's branch, that should allow you to test both changes at once.

Also, I wanted to point out that this PR upgrades the Android Gradle Plugin to 8.4, coming from 7.3.1. That shouldn't be an issue though. This comment points out that it would break users of AGP 4.1, but... we've been mandating AGP >= 7.3.1 for a while now, and #1970, which is the real backwards-incompatible change, was merged in April 2023 with no major complaints.

@MaikuB
Copy link
Owner

MaikuB commented Dec 12, 2024

@Levi-Lesches yep I know. Provided the PR has permissions for it, I could've even pushed changes directly to @martin-bertele's but avoided doing so in case @martin-bertele was going to make changes since you mentioned what can be done already and could I end up wasting @martin-bertele's time. Since we've not heard back though I'll look to do so later if @martin-bertele hasn't gotten to do by the time I can to it

@martin-bertele
Copy link
Contributor Author

Unfortunately I got sick, if I need to change any permissions let me know this I can still do, otherwise I m available next week. Thank you

@MaikuB
Copy link
Owner

MaikuB commented Dec 12, 2024

It's quite possible you ticked it as I believe it's ticked by default but it's to do with this one https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork

@martin-bertele
Copy link
Contributor Author

@Levi-Lesches @MaikuB you can go ahead and copy over any lines from this pr to the work you have, really use the way that works with minimum effort for you. It's been a coincidence that you were to upgrade the sample too.
I got it to run and thought then I ll contribute. :)

AGP and kotlin I was forced to upgrade since there was some compatibility complains.
After that, it built successfully.

Permissions on PR I am not sure how to set, let me know what you need there. Thanks for your great package!

@MaikuB
Copy link
Owner

MaikuB commented Dec 12, 2024

@martin-bertele all good as the permissions are fine as I've been able to push changes directly. Once it's all green and I've done some more testing/reviewing then will merge it in

Copy link
Contributor

@Levi-Lesches Levi-Lesches left a comment

Choose a reason for hiding this comment

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

Compiles on Flutter 3.27.0 with no issues or warnings!

@Levi-Lesches Levi-Lesches mentioned this pull request Dec 13, 2024
@MaikuB MaikuB merged commit f6388f4 into MaikuB:master Dec 14, 2024
17 checks passed
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.

3 participants