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

Merge "mapbox-gl-style-spec" repository into this repository #3946

Merged
merged 1,285 commits into from
Feb 1, 2017

Conversation

lucaswoj
Copy link
Contributor

@lucaswoj lucaswoj commented Jan 11, 2017

This PR merges the "mapbox-gl-style-spec" repository into this repository.

fixes #3794

Todo Before Merging

  • briefly describe the changes in this PR
  • fix eslint failures
  • fix unit test failures
  • fix test suite failures
  • audit all new dependencies to ensure they're all needed
  • change default test reporter
  • remove style spec from package.json & wire GL JS up to bundled style spec version
  • integrate build-style-spec-docs script into build-docs scripts
  • eliminate build-style-spec script in favor of custom browserify middleware
  • add a build test to ensure only the latest minified style spec reference JSON is pulled into the minified bundle
  • set up mapbox-gl-style-spec package to be published from this repo, rename npm package to use new namespace (“@mapbox/…”), write docs on when/how to publish a new style spec version
  • eliminate build-style-spec-api-docs script in favor of JSDoc flow, ensure style spec modules don't show up in GL JS API docs
  • test all bin commands
  • rewrite style spec README
  • manually test the debug page
  • add bubleify to style spec's package.json
  • review all npm packages that depend on style spec
  • pull in all changes from mapbox-gl-js
  • pull in all changes from mapbox-gl-style-spec
  • update docs/_layouts/docs.html with new style spec links

Todo Immediately After Merging

  • publish @mapbox/mapbox-gl-style-spec npm package
  • mark the mapbox-gl-style-spec npm package as deprecated
  • migrate mapbox-gl-style-spec issues & PRs
  • mark the mapbox-gl-style-spec github repo as deprecated
  • sync master and mb-pages
  • set up redirects from the old mapbox-gl-style-spec website

Follow-Up PR

  • move all issues from mapbox-gl to this repo
  • rename this repo to mapbox-gl
  • rename s3 bucket mapbox-gl
  • set up redirects from mapbox-gl-js website to mapbox-gl website

tristen and others added 30 commits January 5, 2016 11:18
 - changes the interface of functions to `f(globals, feature)`
 - removes the "assert" function
 - improves perf
 - pins to an updated style spec
 - bumps to the v2.1.0
V8 is unable to evaluate large disjunctions, failing with "RangeError: Maximum call stack size exceeded".

Fixes #1782
This reverts commit e37aa5f.

Adding validation of ramp stops was good in spirit, but it breaks some v8
styles. Reverting until v9 where we can make backwards incompatible
changes.
Revert "prevent ramp from having duplicate stops"
@anandthakker
Copy link
Contributor

There is almost a third option: we could just export the style spec off of the mapboxgl object, since that would leverage the browserify transform pipeline that's already happening when the main mapbox-gl module is built. However, mapbox-gl minifies the spec, stripping out the documentation, and I'm pretty sure Studio needs that stuff.

Hmm... what if we stuck a check for process.env.MGL_INCLUDE_SPEC_DOCS in minify_style_spec.js and then exported the spec & utils from the mapboxgl object? That would allow Studio and other web-based dependents to do:

  • MGL_INCLUDE_SPEC_DOCS=1 existing_browserify_command ...
  • and just change require('mapbox-gl-style-spec') to require('mapbox-gl').styleSpec.{reference,validate,...}

Not sure if this is actually any better than option 2, though.

@lucaswoj
Copy link
Contributor Author

Thanks for raising these concerns @anandthakker @tmcw. You've brought to light some points I hadn't considered. My plan of attack:

  • add a "browserify" field to style spec's package.json which can reliably transform the package into vanilla javascript (done in cc80268)
  • review public npm packages that depend on style spec, work with the authors of any which use style spec's utility functions, do not use browserify, and need to be run in the browser

}
}
],
"unassertify",
Copy link
Contributor

Choose a reason for hiding this comment

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

Bad trailing comma.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed 👍

@jfirebaugh
Copy link
Contributor

Integration in gl-native looks good: mapbox/mapbox-gl-native@c9f3850.

jfirebaugh and others added 2 commits January 30, 2017 16:45
This turned out to add significant complexity to the gl-native implementation, and never took effect in gl-js due to a bug (#3717). Removing it due to the complexity it would add to gl-native and other implementations, and the following additional rationales:

* As a general principle: explicit is better than implicit.
* Most users will not be constructing functions by hand anyway. For those that do:
* Requiring `"type": "categorical"` is not an undue burden, especially when coupled with a hinting mechanism in the validator as implemented here.
* Implicitly supplying `"type": "categorical"` for string domain values introduces a potentially surprising difference between `"stops": [["1", 1], ["2", 2]]` and `"stops": [[1, 1], [2, 2]]`.
@lucaswoj lucaswoj force-pushed the merge-style-spec branch 6 times, most recently from 643e9fa to 293f49d Compare February 1, 2017 00:21
@lucaswoj lucaswoj merged commit e207068 into master Feb 1, 2017
@lucaswoj lucaswoj deleted the merge-style-spec branch February 1, 2017 01:34
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.

Merge the "mapbox-gl-style-spec" repo into this repo