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

Default max zoom 22#9835

Merged
asheemmamoowala merged 1 commit into
masterfrom
default-max-zoom-22
Aug 22, 2017
Merged

Default max zoom 22#9835
asheemmamoowala merged 1 commit into
masterfrom
default-max-zoom-22

Conversation

@asheemmamoowala
Copy link
Copy Markdown
Contributor

@asheemmamoowala asheemmamoowala commented Aug 22, 2017 •
edited
Loading

Update iOS documentation and use DEFAULT_MAX_ZOOM constant.
+CHANGELOGs

1ec5
1ec5 approved these changes Aug 22, 2017
View reviewed changes
Copy link
Copy Markdown
Contributor

@1ec5 1ec5 left a comment

Choose a reason for hiding this comment

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

Can you also add an entry to the Android, iOS, and macOS changelogs about the increased maximum maximum? Thanks!

Comment thread platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapboxMap.java Outdated
Copy link
Copy Markdown
Contributor

@1ec5 1ec5 Aug 22, 2017

Choose a reason for hiding this comment

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

Typo: “zoomn”.

Comment thread platform/ios/src/MGLMapView.h Outdated
Copy link
Copy Markdown
Contributor

@1ec5 1ec5 Aug 22, 2017

Choose a reason for hiding this comment

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

Property names should be in backticks to benefit from jazzy’s autolinking feature. However, let’s say “default value of this property” here for consistency.

Copy link
Copy Markdown
Contributor Author

@asheemmamoowala asheemmamoowala Aug 22, 2017

Choose a reason for hiding this comment

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

@1ec5 - In this case its referring to itself, so this shouldn't need any linking.

Copy link
Copy Markdown
Contributor

@1ec5 1ec5 Aug 23, 2017

Choose a reason for hiding this comment

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

Yeah, that’s why I suggested saying “default value of this property”, same as what you had written for macOS. No big deal though.

@1ec5 1ec5 mentioned this pull request Aug 22, 2017
Merged
@friedbunny
Copy link
Copy Markdown
Contributor

friedbunny commented Aug 22, 2017

Are there any prerequisites or blockers that would keep these maxzoom changes from being cherry-picked into the current release branch?

@asheemmamoowala
Copy link
Copy Markdown
Contributor Author

asheemmamoowala commented Aug 22, 2017

@friedbunny - I think #9820 and #4168 (for Android only) should be looked at to make sure they are either acceptable or not an issue.

@asheemmamoowala asheemmamoowala merged commit f2c9f2b into master Aug 22, 2017
@asheemmamoowala asheemmamoowala deleted the default-max-zoom-22 branch August 22, 2017 23:50
@friedbunny friedbunny added Android Mapbox Maps SDK for Android Core The cross-platform C++ core, aka mbgl iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS Node.js node-mapbox-gl-native labels Aug 23, 2017
@Guardiola31337 Guardiola31337 mentioned this pull request Oct 6, 2017
Closed
20 tasks
@Guardiola31337 Guardiola31337 added this to the android-v5.2.0 milestone Oct 6, 2017
@Guardiola31337 Guardiola31337 mentioned this pull request Oct 19, 2017
Closed
20 tasks
@Guardiola31337 Guardiola31337 mentioned this pull request Oct 26, 2017
Closed
20 tasks
@tobrun tobrun mentioned this pull request Nov 3, 2017
Closed
21 tasks
This was referenced Nov 14, 2017
Closed
Closed
@1ec5 1ec5 mentioned this pull request Jan 5, 2018
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@1ec5 1ec5 1ec5 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Android Mapbox Maps SDK for Android Core The cross-platform C++ core, aka mbgl iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS Node.js node-mapbox-gl-native

Projects

None yet

Milestone

android-v5.2.0

Development

Successfully merging this pull request may close these issues.

4 participants

@asheemmamoowala @friedbunny @1ec5 @Guardiola31337