Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[android_alarm_manager] Android Code Inspection and Clean up #3020

Merged
dnfield merged 9 commits into from
Sep 14, 2020
Merged

[android_alarm_manager] Android Code Inspection and Clean up #3020

dnfield merged 9 commits into from
Sep 14, 2020

Conversation

@hamdikahloun
Copy link
Member

@hamdikahloun hamdikahloun commented Sep 11, 2020 •
edited
Loading

Description

  • Handle deprecation & unchecked warning as error

  • Avoiding uses or overrides a deprecated API in FlutterBackgroundExecutor.java

  • 'while' loop replaced with enhanced 'for'

  • 'if' statement replaced with 'switch' statement

  • Remove Unnecessary 'Integer.toString()' call

A message is displayed when building for android:

flutter build apk
Running "flutter pub get" in example...                             1,0s
You are building a fat APK that includes binaries for android-arm, android-arm64, android-x64.
If you are deploying the app to the Play Store, it's recommended to use app bundles or split the APK to reduce the APK
size.
    To generate an app bundle, run:
        flutter build appbundle --target-platform android-arm,android-arm64,android-x64
        Learn more on: https://developer.android.com/guide/app-bundle
    To split the APKs per ABI, run:
        flutter build apk --target-platform android-arm,android-arm64,android-x64 --split-per-abi
        Learn more on:  https://developer.android.com/studio/build/configure-apk-splits#configure-abi-split
Note: C:\Users\hamdi\Downloads\plugins-master\packages\android_alarm_manager\android\src\main\java\io\flutter\plugins\androidalarmmanager\FlutterBackgroundExecutor.java uses or overrides a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
Note: C:\Users\hamdi\AppData\Local\Pub\Cache\hosted\pub.dartlang.org\espresso-0.0.1+5\android\src\main\java\androidx\test\espresso\flutter\EspressoFlutter.java uses unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.
Removed unused resources: Binary resource data reduced from 623KB to 581KB: Removed 6%
Running Gradle task 'assembleRelease'...
Running Gradle task 'assembleRelease'... Done                     104,3s
√ Built build\app\outputs\flutter-apk\app-release.apk (16.4MB).

flutter build appbundle
Running "flutter pub get" in example...                             1,1s
Note: C:\Users\hamdi\Downloads\plugins-master\packages\android_alarm_manager\android\src\main\java\io\flutter\plugins\androidalarmmanager\FlutterBackgroundExecutor.java uses or overrides a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
Note: C:\Users\hamdi\AppData\Local\Pub\Cache\hosted\pub.dartlang.org\espresso-0.0.1+5\android\src\main\java\androidx\test\espresso\flutter\EspressoFlutter.java uses unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.
Removed unused resources: Binary resource data reduced from 461KB to 405KB: Removed 12%
Running Gradle task 'bundleRelease'...
Running Gradle task 'bundleRelease'... Done                        78,5s
√ Built build\app\outputs\bundle\release\app.aab (16.5MB).

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • My PR includes unit or integration tests for all changed/updated/fixed behaviors (See Contributor Guide).
  • All existing and new tests are passing.
  • I updated/added relevant documentation (doc comments with ///).
  • The analyzer (flutter analyze) does not report any problems on my PR.
  • I read and followed the Flutter Style Guide.
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy.
  • I updated CHANGELOG.md to add a description of the change.
  • I signed the CLA.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

  • Yes, this is a breaking change (please indicate a breaking change in CHANGELOG.md and increment major revision).
  • No, this is not a breaking change.

dnfield
dnfield reviewed Sep 12, 2020
View reviewed changes
packages/android_alarm_manager/android/gradle.properties Outdated
@@ -1 +1,2 @@
org.gradle.jvmargs=-Xmx1536M
android.enableR8=true
Copy link
Contributor

@dnfield dnfield Sep 12, 2020

Choose a reason for hiding this comment

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

I don't think we want this as part of this patch

Copy link
Member Author

@hamdikahloun hamdikahloun Sep 12, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

it's good, android.enableR8=true removed

dnfield
dnfield reviewed Sep 12, 2020
View reviewed changes
packages/android_alarm_manager/android/gradle/wrapper/gradle-wrapper.properties Outdated
@@ -0,0 +1,5 @@
distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
Copy link
Contributor

@dnfield dnfield Sep 12, 2020

Choose a reason for hiding this comment

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

Did you mean to add this file?

Copy link
Member Author

@hamdikahloun hamdikahloun Sep 12, 2020

Choose a reason for hiding this comment

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

no, i will fix it now and push again , Thank you @dnfield

Copy link
Member Author

@hamdikahloun hamdikahloun Sep 12, 2020

Choose a reason for hiding this comment

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

it's good, I took care of all the comments

dnfield
dnfield reviewed Sep 12, 2020
View reviewed changes
...alarm_manager/android/src/main/java/io/flutter/plugins/androidalarmmanager/AlarmService.java Outdated
Iterator<Intent> i = alarmQueue.iterator();
while (i.hasNext()) {
flutterBackgroundExecutor.executeDartCallbackInBackgroundIsolate(i.next(), null);
/// 'while' loop replaceable with enhanced 'for'
Copy link
Contributor

@dnfield dnfield Sep 12, 2020

Choose a reason for hiding this comment

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

Please remove thes e comments

Copy link
Member Author

@hamdikahloun hamdikahloun Sep 12, 2020

Choose a reason for hiding this comment

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

ah sorry my bad

Copy link
Member Author

@hamdikahloun hamdikahloun Sep 12, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

i juste removed all e commants like

/// 'while' loop replaceable with enhanced 'for'

dnfield
dnfield approved these changes Sep 12, 2020
View reviewed changes
Copy link
Contributor

@dnfield dnfield left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@dnfield dnfield merged commit 4eb849a into flutter:master Sep 14, 2020
@cyanglaz cyanglaz mentioned this pull request Sep 14, 2020
Merged
danielroek pushed a commit to Baseflow/flutter-plugins that referenced this pull request Sep 18, 2020
@hamdikahloun @danielroek
[android_alarm_manager] Android Code Inspection and Clean up (flutter…
5589616 
…#3020)

-Werror and code cleanup for Java code.
jorgefspereira pushed a commit to jorgefspereira/plugins_flutter that referenced this pull request Oct 10, 2020
@hamdikahloun @jorgefspereira
[android_alarm_manager] Android Code Inspection and Clean up (flutter…
80262d7 
…#3020)

-Werror and code cleanup for Java code.
FlutterSu pushed a commit to FlutterSu/flutter-plugins that referenced this pull request Nov 20, 2020
@hamdikahloun
[android_alarm_manager] Android Code Inspection and Clean up (flutter…
3345d94 
…#3020)

-Werror and code cleanup for Java code.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@bkonyi bkonyi Awaiting requested review from bkonyi

1 more reviewer

@dnfield dnfield dnfield approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@hamdikahloun @dnfield @googlebot