Switch JSLint to a submodule

[TomMalbran]

When we went through switching JSLint from a core feature to a default extension, we removed the original submodule and directly uploaded the files. This reverts that by recreating the submodule at the commit of the current files we have.

This will allow us to easily update the version of JSLint later in the future. I haven't done it, since the latest version is checking for new stuff, like unused parameters, unused variables, don't use an else branch after a return, TODO comments, etc. giving warnings in almost every file. So, we would need to update the JSLint flags used on every file and fix other complains.

If we want to get the latest version, which would be good, so we have checks for more stuff (some of which are in JSHint and resolving issue #4462), I would suggest adding the unparam: true and todo: true tags and fix every other issue. unparam: true is required, since when using triggerHandler the callback receives as the first parameter the event, which is almost never used and makes JSLint complain. Making the modules with define(function (require, exports, module) { creates a similar issue, since we don't use all 3 parameters in each module.

[busykai]

I think these are good ideas (make it submodule and bump up the jslint to a newer version). There might be some uncertainty right now -- take a look at the discussion in #4380: it seems that there's an intention to change from JSLint to JSHint.

[dangoor]

@TomMalbran It's a good idea to switch to a submodule. Can you switch to using an https:// URL rather than git:// URL?

[TomMalbran]

@dangoor Done. I just hope the submodule still works.

[dangoor]

Thanks! Looks like it works fine.

[njx]

Hey--fyi, I'm finding that this change from a folder of files to a submodule is making a couple of git workflows hard.

• Checking out branches that haven't merged with this change causes an error, because those branches still have the files in them. I have to manually remove the files each time I switch to one of those branches. That problem will probably go away over time.
• The worse issue is that I can't seem to rebase older branches onto master. The rebase gives me a conflict in the jslint subfolder, and I can't find any way to resolve it. So I have to just merge instead. Again, not the worst thing in the world and will no longer be an issue after awhile, but it means we can't rebase some longer-running branches.

I think this is all just a fact of life, but suggests that replacing an existing folder with a submodule is not well-supported by git, and we should consider trying to avoid it in the future if we get into a similar situation (maybe by naming the submodule something different from the original folder?)

[peterflynn]

@njx I wonder if this fix for a different, but potentially similar, issue would allow you to rebase successfully? http://stackoverflow.com/questions/9974718/git-rebase-and-moving-directory-in-submodule

[njx]

I did end up managing to rebase. I think it has something to do with the fact that when I initially switch to an older branch, I have to delete the jslint files in order to even check it out in the first place. But after that, if I restore the correct version of the files (corresponding to the submodule commit from master), I can get the rebase to work. It was a mess though. Maybe there's some simpler set of steps I could use.

[njx]

Oh, interesting. I guess the idea of the reset is that when you did the merge, it put the submodule reference into your branch and got confused, so resetting sets it back to the original non-submodule-files, which you can then delete, allowing the submodule reference to take over. Maybe? git hurts my brain?

[dangoor]

git submodules are nice in some ways, but there's definitely a large faction of people out there who say "just don't use 'em!" This would be one of those cases.

I like the suggestion to name the submodule something different as a way of avoiding nonsense.

[peterflynn]

@TomMalbran It looks like SHA 2347ec449b would actually be a closer match (actually an exact match) to what we had checked in before this PR. I don't think the slightly-older version at eec32d4b4b is causing any problems, but I'm just curious where that SHA came from?

[TomMalbran]

@peterflynn Yes, you are right. I searched through the commits the first one that had a date of 2011-12-21. But I didn't realized that the dates on the files where not in sync. I took the date 2011-12-21 from the Readme, which is different from the date on jslint.js (2012-01-13).

We can update it if needed, or we can take the job to update to the latest version. Next week I might have some time to work on this.

[dangoor]

@TomMalbran I think we were figuring that it's better to wait for the JSHint switchover rather than touching a whole bunch of files to update JSLint at this time.

[TomMalbran]

@dangoor Ok. I did started working on updating JSLint, and is pretty easy to update the files. Almost every file will have to be updated, but the updates are good, mainly with resolving issues with unused variables and parameters.

[dangoor]

Thanks @TomMalbran. I'll mention that to the team and see if that changes any minds with respect to waiting for JSHint.

[dangoor]

@TomMalbran I spoke with the team about upgrading JSLint and we all agreed that the kind of cleanup you're talking about is all for the better (and better to not wait for the JSHint update). So, feel free to proceed with upgrading JSLint and touching almost every file :)

I'll reopen #4462 since that will benefit from the upgrade.