Build: Migrate to grunt 0.4. #867

jzaefferer · 2012-12-23T11:33:24Z

Rename to Gruntfile, upgrade to newer grunt-css and grunt-html, update custom tasks. Drop qunit-junit plugin, not worth the trouble. Update release script to run grunt-prepare after npm-install.

tree diff between old and new dist:

1c1
< dist

---
> dist-0-3/
22d21
< │   ├── Gruntfile.js
270a270
> │   ├── grunt.js

File diff (diff -r -w -q dist dist-0-3/): https://gist.github.com/fb016517fa530ca75213

The banner creation changed: Whitespace is now more consistent, but slightly different. The "Includes: jquery.ui.[component]" line from non-concat files is gone.

Minifiers changed: grunt-contrib-uglify uses uglify2, grunt-css uses clean-css (instead of squish), so all minified files are different as well.

I've dropped the qunit-junit plugin instead of replacing it with the 0.4 compatible grunt-qunit-junit since that doesn't really work yet and the junit reports have provided no value to us while we were using

I'm done here, but would like someone to review and test before a merge to master.

jzaefferer · 2012-12-23T11:34:09Z

Btw. to test just install grunt-cli globally, then you can npm install when switching branches and get the right local grunt version.

mikesherov · 2012-12-23T13:53:33Z

Gruntfile.js

+				jshintrc: ".jshintrc"
+			},
+			files: {
+				src: [ "grunt.js", "build/**/*.js" ]


Gruntfile.js

mikesherov · 2012-12-23T14:05:11Z

One other note, while you're in build/release.js, you might as well change path.existsSync to fs to get rid of that node warning when you run grunt md5.

mikesherov · 2012-12-23T14:08:15Z

Gruntfile.js

-		return "<strip_all_banners:" + file + ">";
-	});
-}
-
 function stripDirectory( file ) {
 	// TODO: we're receiving the directive, so we need to strip the trailing >


Is this still needed?

Nope, removed.

mikesherov · 2012-12-23T14:17:05Z

Thanks for doing this @jzaefferer ! Also, there was 1 thing that @Krinkle had to modify in Jenkins after we pushed core's upgrade to 0.4. I forgot what it was, but obviously coordinate that with him.

mikesherov · 2012-12-23T14:19:17Z

Last thing to note was, when you add grunt-cli to package.json, you now have a viable npm test command to run, we should consider adding that like we did in core's package.json.

Krinkle · 2012-12-24T04:20:00Z

What I changed in Jenkins is to use ./node_modules/.bin/grunt <cmd> in the Shell Exec build steps of the Jenkins job(s). Since otherwise it will use the global grunt, which is (and should stay for now at) grunt 0.3.x. We want to use the local version anyway for the reason @mikesherov mentioned.

Krinkle · 2012-12-24T04:29:33Z

Actually:

[..] executables will be added to the PATH for executing the scripts.
[..] exported into the node_modules/.bin directory on npm install.

– Path - scripts - NPM Doc

Testing with scripts.install = "echo $PATH;" I see that the following is added to the PATH for npm scripts:

:/usr/local/bin
:/usr/local/lib/node_modules/npm/bin/node-gyp-bin
:/path/to/your-project/node_modules/.bin
:$PATH

So inside the scripts.test property we don't need to prefix ./node_modules/.bin, unless a global version may exist that can conflict (since that has preference apparently).

scottgonzalez · 2012-12-24T13:38:29Z

What I changed in Jenkins is to use ./node_modules/.bin/grunt in the Shell Exec build steps of the Jenkins job(s). Since otherwise it will use the global grunt, which is (and should stay for now at) grunt 0.3.x. We want to use the local version anyway for the reason @mikesherov mentioned.

Why is that the case? Why do we not want grunt-cli in there globally right now? I really don't think we should be putting grunt-cli in our dependencies.

scottgonzalez · 2012-12-24T13:46:28Z

One thing I've been waiting for is the name change from lint -> jshint. Now that we've got it, I'd like to add a new lint task that lints CSS, HTML, JS.

scottgonzalez · 2012-12-24T13:49:03Z

I'll try to test this out this week. Still knee-deep in site work right now.

jzaefferer · 2012-12-24T17:14:06Z

Regarding the lint task: Let's make that the default? Or just add it as an extra task? Order should probably be js, css, html.

scottgonzalez · 2012-12-24T17:50:05Z

I'm fine with making it the default (I'd still like a named task for it though), unless someone strongly objects to removing tests from the default task.

mikesherov · 2012-12-24T22:40:20Z

I like running the tests in the default task personally. But I'd be just as fine removing it.

mikesherov · 2012-12-28T17:47:08Z

Can we land this and then make the minor improvements later? I'm anxious to see this working.

scottgonzalez · 2012-12-28T17:50:33Z

@mikesherov I don't want to land this until I've had time to do a dry run of the release process. That should happen early next week (I'm currently knee-deep in site work).

mikesherov · 2012-12-28T18:08:14Z

no problem. I've got an itchy trigger finger.

jzaefferer · 2013-01-20T21:52:50Z

Just tested and updated this with rc6 and a rebase against master, no changes required for the new rc, just one merge conflict to resolve (in the generate_themes task).

Krinkle · 2013-01-21T23:28:08Z

Gruntfile.js

@@ -18,11 +18,11 @@ var

 	uiFiles = coreFiles.map(function( file ) {
 		return "ui/" + file;
-	}).concat( grunt.file.expandFiles( "ui/*.js" ).filter(function( file ) {
+	}).concat( expandFiles( "ui/*.js" ).filter(function( file ) {


What's with this? Does grunt.file.expandFiles no longer exist? What does it not (or no longer) do that grunt.file.expandMapping (called from expandFiles) does do?

expandFiles is gone and expandMappings returns a map of src -> dest, not an array.

Crap, you're right. It was removed...

jzaefferer · 2013-03-02T12:45:37Z

I've updated to 0.4 final and rebase against master, its working again but also has too much cruft just to migrate to 0.4. I've file an issue against, hopefully we get some help from there: gruntjs/grunt#700

I also want to look into replacing our custom zip task with grunt-contrib-compress, same for grunt-contrib-clean and test the update for grunt-compare-size. Not sure if we should include those here and combine the testing effort, or land this PR asap and do the other updates separately.

mikesherov · 2013-03-05T20:30:24Z

I'm all for land ASAP.

On Sat, Mar 2, 2013 at 7:45 AM, Jörn Zaefferer notifications@github.comwrote:

I've updated to 0.4 final and rebase against master, its working again but
also has too much cruft just to migrate to 0.4. I've file an issue against,
hopefully we get some help from there: gruntjs/grunt#700 gruntjs/grunt#700

I also want to look into replacing our custom zip task with
grunt-contrib-compresshttps://github.com/gruntjs/grunt-contrib-compress/,
same for grunt-contrib-cleanhttps://github.com/gruntjs/grunt-contrib-cleanand test the update for
grunt-compare-sizehttps://github.com/rwaldron/grunt-compare-size/pull/16#issuecomment-14322746.
Not sure if we should include those here and combine the testing effort, or
land this PR asap and do the other updates separately.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/867#issuecomment-14327666
.

Mike Sherov
Lead Developer
SNAP Interactive, Inc.
Ticker: STVI.OB

jzaefferer · 2013-03-09T12:16:56Z

@scottgonzalez are you okay with landing this before 1.10.2? I just did another round of testing and it looks good.

scottgonzalez · 2013-03-11T13:00:16Z

build/tasks/build.js

@@ -5,6 +5,19 @@ module.exports = function( grunt ) {
 var path = require( "path" ),
 	fs = require( "fs" );

+function expandFiles( files ) {


Why is this expandFiles() different than the other? Why don't we just expose it instead of redefining it?

The other one is used directly, and only points at files. This one is used via tasks and gets folder wildcards as inputs. Folders have to be filtered out.

I'm not sure what you mean with "expose it instead of redefining it".

See also gruntjs/grunt#700

scottgonzalez · 2013-03-11T13:02:03Z

If you've tested the release script, then we can land it.

jzaefferer · 2013-03-11T14:40:31Z

The release script still runs "grunt prepare" inside the DB module, but we don't need that anymore, right? I mean, grunt release_cdn works fine as long as you install DB first, but there's no need to run its prepare task. That's the only issue I noticed when running the release script.

scottgonzalez · 2013-03-11T15:40:58Z

Ok, so let's remove the invocation for the prepare task.

Let's just ignore the expandFiles() until later.

…unt-css and grunt-html, update custom tasks. Drop qunit-junit plugin, not worth the trouble. Update release script to run grunt-prepare after npm-install.

jzaefferer · 2013-03-12T10:03:27Z

Probably our fastest build ever, mostly due to the theme generation change. 25 seconds to run the full release_cdn task.

mikesherov reviewed Dec 23, 2012
View reviewed changes

Krinkle reviewed Jan 21, 2013
View reviewed changes

scottgonzalez reviewed Mar 11, 2013
View reviewed changes

jzaefferer added 3 commits March 12, 2013 11:00

Build: Migrate to grunt 0.4. Rename to Gruntfile, upgrade to newer gr…

f87b4e0

…unt-css and grunt-html, update custom tasks. Drop qunit-junit plugin, not worth the trouble. Update release script to run grunt-prepare after npm-install.

Build: Switch from grunt-css to the equivalent grunt-contrib tasks

bd211df

Build: Don't call DB's grunt-prepare task, not needed anymore

ae3af7f

jzaefferer merged commit ae3af7f into master Mar 12, 2013

jzaefferer deleted the grunt-0-4 branch March 12, 2013 10:04

Build: Migrate to grunt 0.4. #867

Build: Migrate to grunt 0.4. #867

Uh oh!

Conversation

jzaefferer commented Dec 23, 2012

Uh oh!

jzaefferer commented Dec 23, 2012

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikesherov commented Dec 23, 2012

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikesherov commented Dec 23, 2012

Uh oh!

mikesherov commented Dec 23, 2012

Uh oh!

Krinkle commented Dec 24, 2012

Uh oh!

Krinkle commented Dec 24, 2012

Uh oh!

scottgonzalez commented Dec 24, 2012

Uh oh!

scottgonzalez commented Dec 24, 2012

Uh oh!

scottgonzalez commented Dec 24, 2012

Uh oh!

jzaefferer commented Dec 24, 2012

Uh oh!

scottgonzalez commented Dec 24, 2012

Uh oh!

mikesherov commented Dec 24, 2012

Uh oh!

mikesherov commented Dec 28, 2012

Uh oh!

scottgonzalez commented Dec 28, 2012

Uh oh!

mikesherov commented Dec 28, 2012

Uh oh!

jzaefferer commented Jan 20, 2013

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jzaefferer commented Mar 2, 2013

Uh oh!

mikesherov commented Mar 5, 2013

Uh oh!

jzaefferer commented Mar 9, 2013

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

scottgonzalez commented Mar 11, 2013

Uh oh!

jzaefferer commented Mar 11, 2013

Uh oh!

scottgonzalez commented Mar 11, 2013

Uh oh!

jzaefferer commented Mar 12, 2013

Uh oh!

Uh oh!