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

[android_intent] Android Code Inspection and Clean up #3043

Merged
merged 13 commits into from
Oct 2, 2020
Merged

[android_intent] Android Code Inspection and Clean up #3043

merged 13 commits into from
Oct 2, 2020

Conversation

hamdikahloun
Copy link
Member

@hamdikahloun hamdikahloun commented Sep 17, 2020

Description

  • Handle deprecation & unchecked warning as error

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

Related Issues

flutter/flutter#65970

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.

@hamdikahloun hamdikahloun changed the title [many] Android Code Inspection and Clean up [android_intent] Android Code Inspection and Clean up Sep 17, 2020
@hamdikahloun hamdikahloun marked this pull request as ready for review September 17, 2020 17:03
Comment on lines 86 to 91
(packageName != null
&& component != null
&& !TextUtils.isEmpty(packageName)
&& !TextUtils.isEmpty(component))
? new ComponentName(packageName, component)
: null;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I would find this easier to read as a regular declaration followed by an if statement. This seems like a lot of logic for a ternary.

bundle.putStringArrayList(key, (ArrayList<String>) value);
} else if (isStringKeyedMap(value)) {
bundle.putBundle(key, convertArguments((Map<String, ?>) value));
} else if (integers(value) != null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I think I prefer the old name of these - I'm a bit surprised to see a function named integers or strings or map.

I also wonder if we can just somehow avoid traversing the array twice and creating the object twice.

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, but needs to have merge conflict(s) fixed up.

/cc @xster FYI as well here

@xster
Copy link
Member

xster commented Sep 26, 2020

If it passes pre-submit then you're good

@hamdikahloun
Copy link
Member Author

@dnfield
merge conflict(s) fixed .
Thank you

@dnfield dnfield merged commit cc9bf02 into flutter:master Oct 2, 2020
yasargil added a commit to yasargil/plugins that referenced this pull request Oct 8, 2020
* master:
  [in_app_purchase] Android Code Inspection and Clean up (flutter#3120)
  Android Code Inspection and Clean up (flutter#3117)
  [in_app_purchase] Fix finishing purchases upon payment cancellation (flutter#3106)
  [google_maps_flutter_web] Fix convert.dart issues (flutter#3093)
  [multiple] Opt-out tests of null-safety (flutter#3113)
  [webview_flutter] add public documentation. (flutter#3114)
  in_app_purchase: started supported null as a parameter for the sandbox arguement (flutter#3110)
  [connectivity] Android Code Inspection and Clean up (flutter#3051)
  [android_intent] Android Code Inspection and Clean up (flutter#3043)
  Remove `io.flutter.embedded_views_preview` from README
  [google_maps_flutter] Fix headline in the readme (flutter#3100)
  [webview_flutter] Add new entrypoint that uses hybrid composition on Android (flutter#3067)
  [google_maps_flutter] Out of developers preview, bump to 1.0.0 (flutter#3091)
  [url_launcher_web] Move third_party under src. (flutter#3080)
  [plugin_platform_interface] Fix homepage in pubspec.yaml (flutter#3088)
  [connectivity_for_web] Fix homepage in pubspec.yaml (flutter#3089)
  [in_app_purchase] Update typo in example main.dart (flutter#3073)
jorgefspereira pushed a commit to jorgefspereira/plugins_flutter that referenced this pull request Oct 10, 2020
FlutterSu pushed a commit to FlutterSu/flutter-plugins that referenced this pull request Nov 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants