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 compile issues in Android (; expected and cannot find symbol) #2184

Merged

Conversation

christoph-jerolimov
Copy link
Contributor

@christoph-jerolimov christoph-jerolimov commented Apr 11, 2018

Does any other open PR do the same thing?

No.

What issue is this PR fixing?

Fixing a compile issue of the Android version. I did not understand exactly what the (merged) PR #2136 does, but it breaks the Android java build with this error(s):

AirMapUrlTileManager.java:50: error: ';' expected
    view.setMinimumZ(minimumZ)

# .. and after fixing this, also here:

AirMapUrlTile.java:32: error: cannot find symbol
      if(this.maximumZ && zoom > maximumZ) {

How did you test this PR?

  • Use the current master of react-native-maps (for example with yarn upgrade react-native-maps@react-community/react-native-maps#master)
  • Build your android app with react-native run-android
  • This fails with the error above. With this changes it compiles again.

@christoph-jerolimov
Copy link
Contributor Author

ping @rborn and @akmjenkins, maybe you can check if the minimumZ > 0 check is what you expect?

@akmjenkins
Copy link
Contributor

akmjenkins commented Apr 11, 2018

@jerolimov How embarrassing, a bunch of things about my previous PR went wrong. Thanks for catching this! I forgot that in Android there are default values specified for minimumZ and maximumZ so do we need the check to see if it's greater than 0 in there at all?

@pedrolopes10
Copy link

Hi, please merge this PR because master is not compiling in Android since 8 days ago!

@rborn
Copy link
Collaborator

rborn commented Apr 18, 2018

@pedrolopes10 can you confirm you tested this PR and it compiles ok ?

@christoph-jerolimov
Copy link
Contributor Author

christoph-jerolimov commented Apr 18, 2018

Checkout the diff. It works and it fixes only obvious syntax errors.. 😉

@rborn rborn merged commit 521e5fe into react-native-maps:master Apr 18, 2018
@pedrolopes10
Copy link

@rborn sorry about the delay, yes I can confirm that it works fine.

Thanks for the merge and the great work!

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.

4 participants