-
Notifications
You must be signed in to change notification settings - Fork 1k
better commonjs test (clean dist, consistent test) #1910
better commonjs test (clean dist, consistent test) #1910
Conversation
- removed commonjs testing resource (js/npm.js) from dist folder - buld dist task no longer leaves dist/js/npm.js in zip - moved creation of resources for commonjs into prep test task - test dist task now preps for commonjs tasks and cleans up
2cd43a7
to
341d2f8
Compare
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.
Fix typo and I think it'll be good to go.
['browserify:commonjs', 'dist', 'jshint', 'connect:testServer', 'qunit:noMoment', 'qunit:globals', 'qunit:dist', 'htmllint', 'resetdist']); | ||
['jshint', 'connect:testServer', 'qunit:noMoment', 'qunit:globals', 'test-dist', 'htmllint']); | ||
|
||
grunt.registerTask('prep-commonjs-test', 'run commonjs config buiild and browserify to prep for commonjs test', |
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.
Typo in description buiild
.
@@ -28,7 +28,7 @@ module.exports = function(grunt) { | |||
|
|||
// Full distribution task | |||
grunt.registerTask('dist', 'Build "dist." Contributors: do not commit "dist."', | |||
['clean:dist', 'distcss', 'copy:fonts', 'copy:templates', 'distjs', 'commonjs', 'distzip']); | |||
['clean:dist', 'distcss', 'copy:fonts', 'copy:templates', 'distjs', 'distzip']); |
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.
Is the commonjs bundle used anywhere else in the dev stuff? Meaning... is it ok that we're removing it from dist completely and only running it during test?
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 it's not used elsewhere
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.
It's used by CommonJS users of Fuel UX and anyone that pulls in FuelUX via NPM.
just wondering, if you're deleting it, will it be re-added during publishing? https://github.com/ExactTarget/fuelux/blob/master/package.json#L74 |
fix #1909