Adds jslint, and linted files. #53

trecenti · 2014-01-21T19:42:52Z

This PR adds jslint using the lib grunt-jslint

here are the directives:

{
browser: true,
indent: 2,
sloppy: true,
nomen: true,
plusplus: true,
predef: [ 'ko', 'jasmine', 'expect', 'describe', 'it', 'spyOn', 'beforeEach', 'afterEach', 'jQuery', '$', 'setFixtures' ]
}

it runs as a dependency for the ci task, or it can be run standalone using grunt jslint.

closes #52

jpbochi · 2014-01-21T19:47:57Z

spec/integration-spec.js

@@ -46,7 +46,7 @@ describe('ko validation integration', function () {
        'integer': [ 'Must be number' ]
      }),
      noNumbersField: ko.observable('').extend({
-        'invalidChars': [ '1234567890'.split(''), 'Must not have numbers.' ]
+        'invalidChars': [ ['1', '2', '3', '4', '5', '6', '7', '8', '9', '0'], 'Must not have numbers.' ]


luizfar · 2014-01-21T19:54:20Z

jpbochi · 2014-01-21T19:56:12Z

src/namespaces.js

@@ -1,4 +1,5 @@
-var ko = ko || {};
+var ko;
+ko = ko || {};


jpbochi · 2014-01-21T20:14:22Z

spec/integration-spec.js

@@ -46,7 +46,7 @@ describe('ko validation integration', function () {
        'integer': [ 'Must be number' ]
      }),
      noNumbersField: ko.observable('').extend({
-        'invalidChars': [ '1234567890'.split(''), 'Must not have numbers.' ]
+        'invalidChars': [ String.prototype.split.call(['1234567890'], ''), 'Must not have numbers.' ]


have you tried ('1234567890').split('')?

jslint doesn't like that.

On Tue, Jan 21, 2014 at 6:14 PM, João Paulo Bochi
notifications@github.comwrote:

In spec/integration-spec.js:

@@ -46,7 +46,7 @@ describe('ko validation integration', function () {
'integer': [ 'Must be number' ]
}),
noNumbersField: ko.observable('').extend({

'invalidChars': [ '1234567890'.split(''), 'Must not have numbers.' ]

'invalidChars': [ String.prototype.split.call(['1234567890'], ''), 'Must not have numbers.' ]

have you tried ('1234567890').split('')?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/53/files#r9052419
.

Bruno Trecenti
thoughtworks.com

jslint doesn't like that.

and I don't like jshint for that 😒

why are we not using jshint?

because of this https://plus.google.com/+DouglasCrockfordEsq/posts/F3SM7uoVo7x

+1 jslint

I don't think anyone agrees with everything in it but I think once you open up the ability to pick and choose it leads to harder team conversations about what you take and what you don't

jpbochi · 2014-01-21T20:52:48Z

then

apretto · 2014-01-21T20:56:52Z

src/validators/utilities.js

-      }
-    }
-  };
+  self.objectForEach = ko.utils.objectForEach;


I say remove self.objectForEach and replace with ko.utils.objectForEach everywhere.

Mixing usage of self.objectForEach with ko.utils.arrayFilter, ko.utils.arrayMap and the likes makes no sense.

I agree, can we do this as part of another PR, there's already a lot of changes not related to jslint here, and I would like to strict to that scope.

apretto · 2014-01-21T20:59:16Z

🚢 🇮🇹

Adds jslint, and linted files.

Bruno Trecenti added 22 commits January 21, 2014 15:57

added js lint taksk

4271884

linted ko-validation.js

db7193e

linted namespaces.js

9c68d36

linted validators-registry.js

001dcb4

linted operators

61a9ea6

linted email-validator

f1d7957

linted only-if-validator.js

618a1c7

linted results.js

2b364b3

linted utiltilies.js

49b7e40

linted computed-observable-spec.js

cb5f7bc

linted integration-spec.js

55fbcf0

linted observable-spec.js

ff898b3

linted validates-spec.js

59bfd65

linted validation-element-spec.js

657f8ee

linted custom-validator-spec.js

f4c95d8

linted function(

cb7f1b6

linted integer-value-validator-spec.js

577300f

linted lenght-validator-spec.js

a3a652d

linted utilities-spec.js

1210171

linted view-model-spec.js

3870d4a

fixed failing tests

a1460bc

added jslint task as a dependency to the ci task

63b3d40

jpbochi reviewed Jan 21, 2014
View reviewed changes

src/namespaces.js

@@ -1,4 +1,5 @@

var ko = ko || {};

var ko;

ko = ko || {};

Copy link

Contributor

jpbochi Jan 21, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😒

Bruno Trecenti added 2 commits January 21, 2014 18:04

avoiding the split jslint error

71d828b

using ko.utils on loops

c0fc930

jpbochi reviewed Jan 21, 2014
View reviewed changes

apretto reviewed Jan 21, 2014
View reviewed changes

trecenti added a commit that referenced this pull request Jan 21, 2014

Merge pull request #53 from racker/js_lint

4dfe442

Adds jslint, and linted files.

trecenti merged commit 4dfe442 into master Jan 21, 2014

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds jslint, and linted files. #53

Adds jslint, and linted files. #53

trecenti commented Jan 21, 2014

jpbochi Jan 21, 2014

luizfar commented Jan 21, 2014

jpbochi Jan 21, 2014

jpbochi Jan 21, 2014

trecenti Jan 21, 2014

trecenti Jan 21, 2014

jpbochi Jan 21, 2014

trecenti Jan 21, 2014

jpbochi Jan 21, 2014

trecenti Jan 21, 2014

tildedave Jan 21, 2014

jpbochi commented Jan 21, 2014

apretto Jan 21, 2014

trecenti Jan 21, 2014

apretto Jan 21, 2014

apretto commented Jan 21, 2014

Adds jslint, and linted files. #53

Adds jslint, and linted files. #53

Conversation

trecenti commented Jan 21, 2014

Choose a reason for hiding this comment

luizfar commented Jan 21, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jpbochi commented Jan 21, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

apretto commented Jan 21, 2014