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

Reduce Android compiler warnings, prevent NPE #963

Merged
merged 4 commits into from
Feb 22, 2019

Conversation

ened
Copy link
Contributor

@ened ened commented Dec 6, 2018

RFC for the FlutterFire team.
I understand that some plugin methods would not typically be called without a FirebaseUser instance, yet the plugin could should probably safeguard against misuse.

Also, as some of the type warnings caused unnecessary logs, I added annotations to prevent the warnings.

@ened ened force-pushed the firebase-auth-warnings branch from 145dc96 to 0873e4f Compare December 6, 2018 02:36
Copy link
Contributor

@mklim mklim left a comment

Choose a reason for hiding this comment

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

LGTM with one minor nit. Thanks for the contribution! If you don't have the time to keep working on this PR, please let us know and we can shepherd it forward.

@@ -265,11 +265,17 @@ public void onCodeAutoRetrievalTimeOut(String verificationId) {

private void handleLinkWithEmailAndPassword(
MethodCall call, Result result, FirebaseAuth firebaseAuth) {
if (firebaseAuth.getCurrentUser() == 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: Can we store this as a local variable so we don't need to fetch it again below? This applies to all of these safeguards.

@ened ened force-pushed the firebase-auth-warnings branch from d756dd0 to 2748984 Compare February 22, 2019 04:43
@mklim mklim merged commit c2b9d52 into flutter:master Feb 22, 2019
julianscheel pushed a commit to jusst-engineering/plugins that referenced this pull request Mar 11, 2020
* Reduce Android compiler warnings, prevent NPE

* Upgrade Android gradle plugin to 3.3.1

* Save few object allocations (nit)
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.

3 participants