Simplify getGruntConfig in how it retrieves the files to lint

[zepumph]

Currently there is a ton of conditional logic to determine whether or not a file should be linted for specific tasks. @samreid and I were talking in regards to #1095 and we think that this can be improved.

Shouldn't we just be able to lint all files in the correct repos for the task, and have the built in eslint directives ( like `// eslint-disable') decide on a per file basis if we want to lint it.

[zepumph]

Maybe rather than using the grunt plugin, we could just use npm's eslint modules. See here http://eslint.org/docs/developer-guide/nodejs-api#getformatter. This could be very similar to how we got boxed in with grunt's requirejs plugin.

[samreid]

I added a new linter above which is simpler in its implementation and more flexible:

• helped catch lint errors the other process was missing
• automatically hits all *.js files instead of just cherry picking some of the js/ directories
• replaced 170 lines of somewhat complicated getGruntConfig.js with 89 lines of more straightforward lint.js
• we need to put 'eslint' in package.json instead of 'grunt-eslint' (seems better to not rely on a plugin being maintained)
• one cache file per target
• doesn't enumerate the "everything" unless you ask for it (speeds up builds)

To use it, replace the lint tasks in Gruntfile.js with these lines:

var lint = require( '../../../chipper/js/grunt/lint' ); // and this import
//...
  /**
   * Returns a function that lints the specified target.
   * @param {string} target 'dir'|'all'|'everything'
   * @returns {function}
   */
  var runLint = function( target ) {
    return function() {
      lint( grunt, target, buildConfig );
    };
  };
  grunt.registerTask( 'lint', 'lint js files that are specific to this repository', runLint( 'dir' ) );

  grunt.registerTask( 'lint-all', 'lint all js files that are required to build this repository', runLint( 'all' ) );

  grunt.registerTask( 'lint-everything', 'lint all js files that are required to build this repository', runLint( 'everything' ) );

You may also need "eslint" in a node_modules, perhaps from chipper/package.json + npm install (somehow I already had one).

After code review and testing is complete, we will need to do the following steps:

1. remove grunt-eslint from package.json
2. add eslint to package.json
3. delete getGruntConfig.js and its usages.
4. add the grunt.registerTasks to master

@pixelzoom, @jonathanolson or @zepumph are you available to review?

[jonathanolson]

Initial review of the code looks good.

Testing this in each repo would be next. I don't see information about what eslint version to use in package.json/etc.

@samreid, could we collaborate on this sometime by changing master working copies, running tests in each repository, and then pushing?

[samreid]

@jonathanolson I'm available today, let me know when is a good time for you to collaborate.

[jonathanolson]

Whenever, also available today. Message me on Skype?

[samreid]

@jonathanolson and I looked into this today, and found the new process seems like it is working well. We tested grunt lint on active runnables and didn't see any new errors (though a few repos were missed because I didn't have npm install already). Next steps:

• commit the changes to master
• remove grunt-eslint from all package.jsons
• check on bayes continuous testing to see if any other errors pop up.
• other steps noted in Simplify getGruntConfig in how it retrieves the files to lint  #565 (comment)
• rewrite findDuplicates

I'll take it from here.

[samreid]

I also realized grunt.initConfig( gruntConfig ); would now be passing in an empty object, so perhaps we don't need to call it any more.

[samreid]

Find duplicates didn't "just work" with the new paths, so I inlined getGruntConfig into it (so it will only be called when needed) and created #566.

[samreid]

I commented in the skype channel:

Brace yourself for a mega commit, 100+ repos

[samreid]

I commented in skype:

pushes complete, JO and I tested locally and all seemed well, but I recommend you wait for bayes to take a closer look before you pull

[samreid]

I commented in skype:

After pulling the 100+ repo changes, you (everyone) will need to npm prune + npm update in all your repos.

[samreid]

Bayes helped catch 3 errors, everything else seems OK. We should also keep our eyes peeled for perennial issues since perennial/package.json changed.

[samreid]

I wrote in on skype:

[4/17/17, 11:08:46 AM] Sam Reid: Dear next deployer, perennial/package.json changed and the next RC or production deploy will help us understand whether it is OK
[4/17/17, 11:08:53 AM] Sam Reid: So keep you eyes peeled!

[samreid]

I added:

[4/17/17, 11:10:37 AM] Sam Reid: I have no idea if/how perennial updates itself. Do we have to pull it manually? Or does it always self-pull?
[4/17/17, 11:10:50 AM] Sam Reid: Does it know how to npm prune + npm update itself?

[samreid]

It sounds like perennial update is manual, and dev release was made with no trouble. I'll close this until we run into problems.