-
-
Notifications
You must be signed in to change notification settings - Fork 306
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
Removed submodule maplibre-gl-js #938
Removed submodule maplibre-gl-js #938
Conversation
Updated expected image output for 2 failing tests because of the antialiasing.
…eference and shaders to this repo.
The style specification file documents internally which platforms support which properties. If we copy the style specification file into this repository, we will have multiple versions and it might be unclear which style specification file is the source of truth for supported platforms across MapLibre. @HarelM @birkskyum should we move the documentation of the supported platforms out of the style specification file? |
As a temporary solution, this is OK, but I think the style spec repo should either create a package that this repo can use to get the content of the style spec, or simple get the spec file from the style-spec npm package. |
Very nice @alexcristici . Could you motivate this change? I know this was discussed privately but for the record it would be good to have the considerations behind PRs public, in this case for removing the
This is temporary until the style spec is published as something we can consume. Ideally we should not have to depend on an npm package in my opinion. |
What makes it difficult to consume the style spec package currently? It's there a link to a discussion around that anywhere |
I don't think that has been discussed yet. But I only see (TypeScript) source code releases. https://github.com/maplibre/maplibre-gl-style-spec |
If you use the repo as a submodule, sure. There are generated js files in the npm package though |
I updated the description with some of the reasons of this change. |
Thanks for the info. |
Thanks for updating the description @alexcristici. Good to know you tried to update. Let us know when this PR is ready for review.
Agreed! |
Ready for review! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI/CD ratified it!
Another meticulous PR. Thanks @alexcristici!
I copied from
maplibre-gl-js
the following folders:src/shaders
->shaders
src/style-spec/reference
->scripts/style-spec-reference
test/integration
->metrics/integration
And updated the scripts and makefiles accordingly.
Some of the reasons behind the decision: