Skip to content

String format with decimal issue in Android 8.1.0 #3151

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

Closed
wants to merge 2 commits into from
Closed

String format with decimal issue in Android 8.1.0 #3151

wants to merge 2 commits into from

Conversation

argzdev
Copy link
Contributor

@argzdev argzdev commented Nov 19, 2021

Speculative fix for issue #3084. NullPointerException is encountered specifically on Android version 8.1.0, this is due to String.format inserted with Double values.

The proposed fix is to use DecimalFormat to format the values before being inserted in String.format

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Nov 19, 2021

Coverage Report

Affected SDKs

No changes between base commit (4e336c6) and head commit (21f5e771).

Test Logs

Notes

HTML coverage reports can be produced locally with ./gradlew <product>:checkCoverage.
Report files are located at <product-build-dir>/reports/jacoco/.

Head commit (21f5e771) is created by Prow via merging commits: 4e336c6 a672e16.

@argzdev
Copy link
Contributor Author

argzdev commented Nov 19, 2021

/retest

@google-oss-bot
Copy link
Contributor

Binary Size Report

Affected SDKs

  • firebase-perf

    Type Base (4e336c6) Head (21f5e771) Diff
    aar 301 kB 301 kB +24 B (+0.0%)
    apk (aggressive) 980 kB 981 kB +124 B (+0.0%)
    apk (release) 2.45 MB 2.45 MB +52 B (+0.0%)

Test Logs

Notes

Head commit (21f5e771) is created by Prow via merging commits: 4e336c6 a672e16.

@argzdev
Copy link
Contributor Author

argzdev commented Nov 22, 2021

/retest

Copy link
Contributor

@visumickey visumickey left a comment

Choose a reason for hiding this comment

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

Since this is speculative fix, the changes LGTM. The changes are harmless even if the fix does not resolve the issue. We can merge as soon as the PR goes green.

I'm working to see what can be done for the CLA.

@argzdev
Copy link
Contributor Author

argzdev commented Nov 26, 2021

Hi @visumickey, thank you for the approval. Unfortunately, after discussing with my team, it looks like there was an issue with the author email(personal email) I used when committing the change, as a result this might cause an issue with the CLA. So in order to avoid this, I created another PR with the correct details:

#3176

I'll close this now. Thank you!

@argzdev argzdev closed this Nov 26, 2021
@firebase firebase locked and limited conversation to collaborators Dec 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants