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

Replacing hardcoded configuration with configurable API and removing Mapbox assets #90

Merged
merged 85 commits into from
Jun 29, 2021

Conversation

petr-pokorny-1
Copy link
Contributor

@petr-pokorny-1 petr-pokorny-1 commented May 28, 2021

This PR removes hardcoded Mapbox configuration and local (as well as references to remote) assets from the project. MapLibre was using Mapbox styles, sources, glyphs, ... so far everywhere in unit tests, integration tests and test application and references to these assets were hardcoded in the codebase. For the future stability of the development environment it was necessary to migrate away from Mapbox infrastructure.The goal of this PR is:

  • Make all tests and test apps independent from Mapbox infrastructure and use MapLibre or MapTiler Cloud instead Mapbox
  • Expose API which allows to provide custom backend (tile server) configuration or choose predefined one.
  • Replace hardcoded style URLs such us Style.STREETS with configurable ones
  • Remove references to Mapbox styles and sources from comments
  • Remove remaining Mapbox packages such as mapbox-android-account
  • Remove map feedback links to Mapbox servers

Most changes introduced by this PR are not visible publicly because they were made in unit/integration tests and test apps. The publicly visible changes are:

  1. Android
  • Startup. Original Mapbox SDK requires to call Mapbox getInstance(Context context, String apiKey) before the app instantiates any MapView (or inflates any view). The purpose of this call is to establish connection with the native Mapbox core library and set the globally shared access token. This PR extends the method to accept additional parameter WellKnownTileServer enum which currently has three possible options: MapLibre, Mapbox and MapTiler. For user who don't need any predefined backend, it is possible to use Mapbox.getInstance(Context context).
  • Mapbox predefined styles (such as Style.MAPBOX_STREETS) were removed from Style class and replaced with getPredefinedStyle(String name) method which allows to get style URL using a string key. This method uses the configuration specified on startup.
  1. iOS
  • Startup. On app start (typically in app delegate) it is now possible use [MGLSettings useWellKnownTileServer:]; to configure the remote tile server. It has to be done before the app instantiates any MapView Possible options are: MGLMapLibre, MGLMapbox and MGLMapTiler.
  • Mapbox predefined styles were removed from MGLStyle class and replaced with + (MGLDefaultStyle*) predefinedStyle:(NSString*)withStyleName; method which allows to get style URL using a string key. This method uses the configuration specified on startup.

The predefined tile servers and styles are configured in src/mbgl/util/tile_server_options.cpp class.

The names access_token, mapbox access token etc were replaced with ApiKey everywhere (MGL_API_KEY in scripts) and for local development purposes the api key can be set in ~/maplibre file.

There are still some places in the codebase where mapbox stuff is being referenced or used, but since this PR is huge one, it should be removed in another separate PRs. It will be also necessary to update the documentation.

…d TileServerOptions to relevant classes and method in order to remove mapbox hardocded configuration
…plementing JNI conversion for TileServerOptions
@petr-pokorny-1 petr-pokorny-1 merged commit d62ff40 into master Jun 29, 2021
@petr-pokorny-1 petr-pokorny-1 deleted the tile-server-properties branch June 29, 2021 10:44
@carlonzo
Copy link
Collaborator

thats massive. thank you! :)

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.

2 participants