clean up the build scripts, enforce a bit of eslint, update dependencies

[sulcata]

• Cleaned up the build scripts and got rid of unnecessary clutter. For whatever reason we weren't using load-grunt-tasks before.
• Updated the dependency versions.
• Added ESLlint A large chunk of the rules I enabled for ESLint are purely stylistic and can be changed if people dislike them (is anyone even working on this?). It would be good to just run with a set style though.

There were quite a few errors that I had to stuff by turning certain rules off in a lower down .eslintrc file in app/assets/javascript. We should really turn most of these files into modules and add a browserify step so we can get proper linting of typos along with cleaning up the global namespace, but that's a much larger task for a much farther day. I'll open up an issue or two.

also, i was pleasantly surprised to only find two nested ternaries :]

[sulcata]

@Fuzzysqurl are you familiar enough with Travis to help me set up automatic linting of PRs? I think a maintainer of the repo needs to do something, but I'm not sure.

[turbedi]

Uhmm.. am I doing something wrong?

[sulcata]

"Yes" in that we probably shouldn't be relying on the coercing of double equals all that often. "No" in that I forgot restuff the error before committing so we don't get flooded with obnoxious messages.

[coyotte508]

Uh...

What's the point?

Bonus: ??? Malus: Huge commit, breaks git history for things like indent or even space after braces, etc.

If someone works heavily on a file and wants to 'fix' the indent (although that's kind of impsing your own will!) sure, or if someone edits a lot of files ok, but really it serves no purpose.

Cleaning up the build script and upating the dependencies is great, but really the stylistic thing is actually harmful. Editing thousands and lines for stylistic purpose goes against any guideline for a repository under a versioning system (unless we start version 2.0 of the client or w/e), where we want to edit the least number of lines per commit.

[sulcata]

Whether it's my will or others' wills doesn't really matter that much, we do need some sort of "will". If anyone wants different rules they're more than welcome to of course. The point is that we need a consistent style to make reading and editing lines easier.

Mixing quotes, randomly changing indentation levels, mixing tabs and spaces, nesting ternary, 120+ char lines, inconsistent naming, "undefined" variables, a dozen object properties on a line and so on just makes the code more difficult read and maintain further down the road.

[sulcata]

Also what git guidelines are we even following, because squashing commits all the time probably isn't a generally accepted practice either :x

[coyotte508]

Mixing quotes is harmless, I try to keep indentation to spaces but I'm not gonna edit all the files to fix it because it will break the history of the repo, ternary operator is bad if abused but twice in the repo (and in short lines) is harmless. There are even cases where a line is changed because adding a space before ':' or after '{', those changes really shouldn't be here and just burden the repository.

Regarding undefined variables, sure. If there's dozen of properties in one line, something can be done about it on a case by case basis.

Regarding readabilty, the bracing style is consistent, the code is indented. Readability could be improved with comments or restructuring but those aren't in this commit. Neither is making the naming style consistent in this commit (it isn't already?).

A blanket change like this defeats the point of a versioning system. We may as well erase all past commits and start a band new repository.

(Note: The build system change and dependency upgrade are :thumbs up:)

[coyotte508]

Squashing commits is because we're not too strict on the committers (w can't afford to) and should be done only on messy pull requests. It's a compromise.

[sulcata]

Bracing style wasn't consistent. The code randomly had lines with 1, 3, or 5 spaces of indentation throughout it, and at times either indented too many or too few levels. A small number of function arguments in PokeInfo were using underscores, as well as in the build file. We should probably not be setting any sort of precedent in abusing the ternary operator at all due to how unreadable it gets (those two lines were kind of nasty tbh).

Also forgot to mention that there was more ? true : false and ? false : true than there should have been :/

Mixed quotes and differing object literal styles are messy. Is there any reason to sacrifice readability for the sake of allowing these styles?

If it's really not possible to lint through the main Javascript files now, can we at least leave the linting in the build files? It wouldn't be difficult to simply exclude them for now. We can't get any proper undefined checking until we modularize them anyway. Although modularization would require changing all the files at once as well.

edit: I'd like to note that there's a very huge bonus in linting javascript files since you won't be able to easily tell when something is or is not defined unless the code in that block specifically runs. It'd result in a hell of a lot less debugging time.

[sulcata]

Also for the record most of these changes to the source files were done with eslint's fix option, so I haven't really sunk a whole lot of time into that in particular at least. I figured a large PR would reanimate coyo in time for SuMo though~

[coyotte508]

Well can you make the changes to the build script and dependencies in a separate PR (maybe 2 commits?).

Regarding the ? true : false, they serve a purpose of returning a boolean instead of a value or object.

Regarding quotes and double quotes, things like $('input[type="submit"]') become $("input[type=\"submit\"]"), that's just spitting on the convenience of being able to use either.

And regarding the whole lsinting thing, I'm ok if someone makes change to the webclient, he can edit stuff as he goes along. So you can fix the build files if you want, but it's all spaces indented 2 consistently, so I'm really just going to facepalm if you're still going to reindent and change quotes to double quotes or whatever, but hey, you're cleaning up the build script so w/e.

The bracing style is consistent, at least everywhere I looked it was like this:

if (abc) {
}

Regarding my activity, no promises, kind of busy.

edit: The huge bonus ...? I'm not really seeing any concrete example with PO's code. Or maybe you mean when coding and making a typo and calling a variable not defined? The autocomplete of sublime text lets you avoid typing whole variable names.

[sulcata]

Not when the condition is already a boolean coyo. Not much reason to avoid using !! when we're using it already and it's shorter. Could also use Boolean() if you want to be more clear.

In your example you could simply reverse the quotes instead of escaping double quotes, no?

I don't really see why the build files should differ in style from the rest of the code base. Seems a bit silly.

A few function definitions put the brace on a new line. A few multi-line if-else conditionals omitted the braces. Can't hurt to continue to enforce an already prevalent style.

I don't use sublime, but does its autocomplete account for the scoping of variables?

[sulcata]

I've reduced the scope to exclude the main javascript files. I'll make a separate PR.