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

fix: possible crash #57

Merged
merged 11 commits into from
Jul 12, 2023
Merged

fix: possible crash #57

merged 11 commits into from
Jul 12, 2023

Conversation

tinted-knight
Copy link
Contributor

@tinted-knight tinted-knight commented Jul 6, 2023

Related tasks

Fixes #10 , might also affect #34 , #39 and #51

Changes

  • Setting lastResult field to null when success or error message was delivered to prevent calling it more than once, which causes RuntimeException

Troubles (optional)

  • Saving Result in class field, waiting for BroadcastReceiver callbacks and using it in onActivityResult looks kind on unstable. Fixing it will probably lead to significant changes in the API.

Note (optional)

  • No breaking changes, the purpose is to evade crash, it still will not propagate second received code to Flutter
  • Unstable crash on second received code
  • Unstable crash on timeout

Checklist for self-check

  • Commits and PRs have been filed according to the rules on the project.
  • The author is marked as an assignee and assigned mandatory reviewers.
  • Required labels marked
  • Specified related tasks and/or related PRs.
  • Specified Changes.
  • Attached videos/screenshots demonstrating the fix/feature.
  • All unspecified fields in the PR description deleted.
  • New code covered by tests.

Checklist for reviewers

  • CI passed successfully (with a green check mark).
  • PR is atomic, by volume no more than 400 (+-) corrected lines (not including codegen).

Design:

  • System design corresponds to the agreements on structure and architecture on the project.
  • The code is decomposed into necessary and sufficient components.

Functionality:

  • The code solves the problem.
  • Any changes to the user interface are reasonable and look good.

Complexity:

  • The code is clear, easy to read, functions are small, no more than 50 lines.
  • The logic is not overcomplicated, there is no overengineering (no code sections that may be needed in the future, but no one knows about it).

Tests:

  • Updated or added tests for mandatory components.
  • The tests are correct, helpful, and well designed/developed.

Naming:

  • The naming of variables, methods, classes and other components is understandable.

Comments:

  • The comments are understandable and helpful.

Documentation:

  • All labels are correct
  • Technical documentation updated (after approval, updates last reviewer).

- unstable crash on second received code
- unstable crash on timeout
@tinted-knight tinted-knight force-pushed the fix_second_code_crash branch from be3f7d3 to 85c5b38 Compare July 6, 2023 21:39
@plasticfiresam plasticfiresam changed the base branch from main to feat-update-workflow July 11, 2023 08:36
maxim-sysoev
maxim-sysoev previously approved these changes Jul 11, 2023
Base automatically changed from feat-update-workflow to main July 11, 2023 09:14
@plasticfiresam plasticfiresam dismissed maxim-sysoev’s stale review July 11, 2023 09:14

The base branch was changed.

@codecov-commenter
Copy link

Codecov Report

Patch and project coverage have no change.

Comparison is base (9ee394c) 96.34% compared to head (2895733) 96.34%.

❗ Current head 2895733 differs from pull request most recent head f434c9e. Consider uploading reports for the commit f434c9e to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #57   +/-   ##
=======================================
  Coverage   96.34%   96.34%           
=======================================
  Files           4        4           
  Lines          82       82           
=======================================
  Hits           79       79           
  Misses          3        3           

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@plasticfiresam plasticfiresam added the WIP Work in progress label Jul 11, 2023
@plasticfiresam plasticfiresam changed the base branch from main to dev July 12, 2023 09:08
@maxim-sysoev maxim-sysoev merged commit 1e63cc3 into dev Jul 12, 2023
@maxim-sysoev maxim-sysoev deleted the fix_second_code_crash branch July 12, 2023 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request WIP Work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Android App crashes when phone allowed two SMS at the same time.
6 participants