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

Fix build by installing grunt dependencies later #22534

Merged
merged 1 commit into from
Apr 27, 2017
Merged

Conversation

Johann-S
Copy link
Member

@Johann-S Johann-S commented Apr 27, 2017

Close : #22476

package.json Outdated
@@ -38,7 +38,9 @@
"js-minify": "uglifyjs --compress warnings=false --mangle --comments '/^!/' --output dist/js/bootstrap.min.js dist/js/bootstrap.js",
"js-minify-docs": "uglifyjs --compress warnings=false --mangle --comments '/^!/' --output docs/assets/js/docs.min.js docs/assets/js/vendor/anchor.min.js docs/assets/js/vendor/clipboard.min.js docs/assets/js/vendor/holder.min.js docs/assets/js/src/application.js",
"js-test": "phantomjs ./node_modules/qunit-phantomjs-runner/runner.js js/tests/index.html 60",
"js-test-cloud": "ruby -run -ehttpd . -p3000 > /dev/null & grunt saucelabs-qunit",
"js-test-dep": "npm install grunt && npm install grunt-saucelabs",
"js-launch-cloud": "ruby -run -ehttpd . -p3000 > /dev/null & grunt saucelabs-qunit",
Copy link

@Florian-R Florian-R Apr 27, 2017

Choose a reason for hiding this comment

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

The indent is weird here

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed thank you 😉

@Johann-S Johann-S force-pushed the v4-test-fix-build branch from 060a273 to 4606012 Compare April 27, 2017 09:55
@Johann-S
Copy link
Member Author

I spammed a lot SauceLabs 😂 so they don't want to run our unit test but It seems our build system is OK now ❤️

/CC @mdo and @bardiharborow

@patrickhlauke
Copy link
Member

speaking of fixed build system, am i the only one getting errors relating to markup linting when i try to run the new build system locally? #22500

@Johann-S
Copy link
Member Author

Johann-S commented Apr 27, 2017

I have no errors when I runned : npm run docs-lint

EDIT :
I have the same error when I run npm test
@patrickhlauke

@Florian-R
Copy link

@Johann-S Can you expand of this change, I'm curious to know why you need to install grunt this way.

Also, optional-dev-dependency might clean the code a bit.

@Johann-S
Copy link
Member Author

Because we install those nodes modules just when we want to run our tests on Saucelabs.
The previous error was (see : https://travis-ci.org/twbs/bootstrap/jobs/225503461#L307) :
sh: 1: grunt: not found

So with this way we are sure that grunt is present

@@ -38,7 +38,9 @@
"js-minify": "uglifyjs --compress warnings=false --mangle --comments '/^!/' --output dist/js/bootstrap.min.js dist/js/bootstrap.js",
"js-minify-docs": "uglifyjs --compress warnings=false --mangle --comments '/^!/' --output docs/assets/js/docs.min.js docs/assets/js/vendor/anchor.min.js docs/assets/js/vendor/clipboard.min.js docs/assets/js/vendor/holder.min.js docs/assets/js/src/application.js",
"js-test": "phantomjs ./node_modules/qunit-phantomjs-runner/runner.js js/tests/index.html 60",
"js-test-cloud": "ruby -run -ehttpd . -p3000 > /dev/null & grunt saucelabs-qunit",
"js-test-dep": "npm install grunt && npm install grunt-saucelabs",
Copy link
Member

Choose a reason for hiding this comment

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

Did npm install & ruby -run [...] not work with these as dependencies? Was explicitly installing these the only way it worked?

Copy link
Member Author

@Johann-S Johann-S Apr 27, 2017

Choose a reason for hiding this comment

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

I just want to install these two dependencies that's why, npm install will install all the dependencies.

EDIT :

Sorry didn't understood well your sentence... My bad
Yes, I don't know why but grunt wasn't known as a program before I did that
But I think it's a good thing to remove grunt from devDependencies because we use grunt only to trigger cloud test on Saucelabs

@Johann-S Johann-S merged commit d336adf into v4-dev Apr 27, 2017
@Johann-S Johann-S deleted the v4-test-fix-build branch April 27, 2017 21:03
@Johann-S Johann-S added this to the v4.0.0-beta milestone Apr 27, 2017
@mdo mdo mentioned this pull request Apr 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants