Review JSHint options

[Reinmar]

As #2, but for the JSHint.

Note: We should not enable options which are checked by the JSCS.

[Reinmar]

I pushed my proposal in https://github.com/ckeditor/ckeditor-boilerplate/tree/t/8.

I'm not sure about these rules:

• http://jshint.com/docs/options/#eqeqeq
• http://jshint.com/docs/options/#forin
• http://jshint.com/docs/options/#freeze - not sure, because it's so obvious :D,
• http://jshint.com/docs/options/#maxparams and the reset of max* options,
• http://jshint.com/docs/options/#eqnull
• http://jshint.com/docs/options/#expr
• http://jshint.com/docs/options/#validthis

[fredck]

http://jshint.com/docs/options/#eqeqeq

I don't remember us having any problem with equality and even though jshint shows the dangerous cases so we use them only when necessary (e.g. === 0).

http://jshint.com/docs/options/#forin

I have doubts here as well.

http://jshint.com/docs/options/#freeze - not sure, because it's so obvious :D,

We should include this, just for correctness.

http://jshint.com/docs/options/#maxparams and the reset of max* options,

Nah... let's leave some room for our brains to think :)

I'm just curious about maxcomplexity.

http://jshint.com/docs/options/#eqnull

If I understand it well, I'm ok to not have == null checks.

http://jshint.com/docs/options/#expr

It's unclear for me. Maybe an example would help clarifying it.

http://jshint.com/docs/options/#validthis

Let's wait and see if this will be a pain or not.

[jodator]

http://jshint.com/docs/options/#validthis

Let's wait and see if this will be a pain or not.

It is good when you have to implicitly write that this is valid. It gives some boilerplate - I have some

/* jshint validthis: false */
this.foo();
/* jshint validthis: true */

in code, but this makes you to check whether this is this this and not the other.

[Reinmar]

http://jshint.com/docs/options/#validthis

Let's wait and see if this will be a pain or not.

I've actually never been a big fan of playing with contexts, because in many cases it isn't obvious what the context is. That doesn't of course mean that I don't like this:

editor.on( 'foo', someObj.onFoo, someObj );

In such case, this used in onFoo refers to someObj, so this will not trigger JSHint warnings and this is totally clear.

tl;dr - let's not use the validthis option. Eventually, it will reduce number of cases when someone is playing with contexts.

---

http://jshint.com/docs/options/#expr

It's unclear for me. Maybe an example would help clarifying it.

I'm not sure, actually, but I thought that it's about cases like this (although the description doesn't match... :D):

if ( el = walker.next() ) ...

But I'm ok with limited amount of the above if it's wrapped with additional parentheses. Very often we use:

while ( ( el = walker.next() ) ...

And this is ok. I don't like cases when assignments start to appear in some long conditions like this.

---

As for eqeqeq and forin, I know that many JS devs including its icons hate == and unfiltered for-in loops, but I also never saw a single case when it was a problem.

---

As for eqnull I liked foo == null (which is true for undefined and null) what's often handy, but I know that this "trick" isn't well known, so we can avoid them. But if you think that it's not a WTF, then let's allow these.

---

I'm just curious about maxcomplexity.

Yep, me too. But someone would have to tell me what value is reasonable :D.

[fredck]

But I'm ok with limited amount of the above if it's wrapped with additional parentheses.

Yes, I like this this way as well.

But if you think that it's not a WTF, then let's allow these.

Yes, we can allow this. Lack of js knowledge shouldn't be a decision point for us.

Yep, me too. But someone would have to tell me what value is reasonable :D.

For now let's just keep this as a curiosity in our head. Later on we may play with it with real code and see how it works.

Taking all this in consideration, you can go ahead closing this issue.

[fredck]

Merged it with b7383e7.