-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Ensure style-spec is published with mapbox-gl #6586
Conversation
cc @samanpwbb |
@@ -141,13 +142,14 @@ | |||
"test-suite": "run-s test-render test-query", | |||
"test-suite-clean": "find test/integration/{render,query}-tests -mindepth 2 -type d -not \\( -exec test -e \"{}/style.json\" \\; \\) -print | xargs -t rm -r", | |||
"test-unit": "build/run-tap --reporter classic --no-coverage test/unit", | |||
"test-build": "build/run-tap --no-coverage test/build/**/*.test.js", |
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.
Does this get run in CI?
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.
Oops, good catch -- accidentally omitted the circle.yml change
"build-css": "postcss -o dist/mapbox-gl.css src/css/mapbox-gl.css", | ||
"build-style-spec": "cd src/style-spec && npm run build && cd ../.. && cp -R src/style-spec/dist dist/style-spec", |
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.
Any particular reason for npm
rather than yarn
?
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.
Nope, just habit, will change.
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.
Reverted to npm
due to mapbox/mbgl-ci-images#17
@@ -131,6 +131,8 @@ jobs: | |||
- run: yarn run build-min | |||
- run: yarn run build-dev | |||
- run: yarn run build-css | |||
- run: yarn run build-style-spec | |||
- run: yarn run test-build |
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.
@jfirebaugh do you see any reason to do test-build
as a separate workflow step? I thought it kinda made sense to keep it here, but 🤷♂️
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.
No, adding it here is what I would have done too.
1a4541c
to
b8c34aa
Compare
Cherry pick 478dd56 from master * Ensure style spec is published with build Closes #6433 Closes #6574 * Consolidate build unit test commands * Workaround CI image path bash/sh issue mapbox/mbgl-ci-images#17
* Fix style-spec build (#6575) * Ensure style-spec is published with mapbox-gl (#6586) Cherry pick 478dd56 from master * Ensure style spec is published with build Closes #6433 Closes #6574 * Consolidate build unit test commands * Workaround CI image path bash/sh issue mapbox/mbgl-ci-images#17 * Bump style-spec version to 12.0.0-pre.1 (#6595) * Use named exports in style-spec.js (#6602) Cherry pick e83054f from master Fixes #6601 * Ignore whole /dist/ directory Done in master with e83054f
Includes a copy of the compiled style-spec bundle under
dist/style-spec/index.js
in the published mapbox-gl package