Skip to content

Conversation

@DavidMina96
Copy link
Contributor

Description of the change

Problem:

Users face a compile error on Android stating that local variables accessed from within inner class need to be declared final. This is only encountered on projects with Java version prior to 8.

Starting Java 8, this requirement was relaxed to final or effectively final. A variable whose value is never changed after it is initialized is effectively final. Hence, users on Java 8 or higher do not face this issue.

Solution:

Declare these local variables explicitly final so that the app builds successfully on different Java versions.

Type of change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Related issues

Issue links go here

Checklists

Development

  • Lint rules pass locally
  • The code changed/added as part of this pull request has been covered with tests

Code review

  • This pull request has a descriptive title and information useful to a reviewer
  • Issue from task tracker has a link to this pull request

TheBuggedYRN
TheBuggedYRN previously approved these changes Oct 3, 2022
Copy link
Contributor

@a7medev a7medev left a comment

Choose a reason for hiding this comment

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

I think also we need to do the same thing to RNInstabugReactnativeModule.setInvocationEvents parameters.

@DavidMina96
Copy link
Contributor Author

@a7med-mahmoud The compiler didn't complain about it, but yes we can better do the same here.

a7medev
a7medev previously approved these changes Oct 3, 2022
TheBuggedYRN
TheBuggedYRN previously approved these changes Oct 3, 2022
Co-authored-by: Ali Abdelfattah <aabdelfattah@instabug.com>
@codecov
Copy link

codecov bot commented Oct 3, 2022

Codecov Report

Merging #788 (96feec2) into master (792dc64) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #788   +/-   ##
=======================================
  Coverage   89.98%   89.98%           
=======================================
  Files          23       23           
  Lines         599      599           
  Branches       93       93           
=======================================
  Hits          539      539           
  Misses         52       52           
  Partials        8        8           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ymabdallah ymabdallah merged commit a552543 into master Oct 3, 2022
@ymabdallah ymabdallah deleted the fix/declare-local-vars-final branch October 3, 2022 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants