Opened 9 years ago

Closed 7 years ago

Last modified 7 years ago

#11554 closed enhancement (fixed)

JSLint Fork, should we use it?

Reported by: dylan Owned by:
Priority: high Milestone: 2.0
Component: General Version: 1.5
Keywords: Cc:
Blocked By: Blocking:

Description

Read: http://mikewest.org/2010/05/jslint-needs-some-bad-parts Should we switch Dojo to using http://github.com/mikewest/jslint/ instead of the version of JSLint we currently use? Or even our own fork for the things that always make us just commit with !strict ?

Attachments (1)

jsl.conf (6.1 KB) - added by dante 9 years ago.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 9 years ago by bill

ISTM that fork is just changing how JSLint checks CSS. I didn't even know it checked CSS, but in any case, we aren't using it for that. Did I miss something?

comment:2 Changed 9 years ago by James Burke

I thought the commit thing was just a spidermonkey or rhino strict check, not something using JSLint. Pete said he had a configured jslint he uses, all the checks in JSLint are configurable, either place the config in the files as comments or set command line args, iirc. So I think we can use the real one if we want, just have to set up the config correctly.

However, I like using the basic one with no config, or just a small one in each individual file, normally with nomen: false, plusplus:false, but that is a bit more extreme compared to Dojo's current code standard. But I like it for a 2.0 :)

comment:3 Changed 9 years ago by dante

yah the current pre-commit hook is just rhino strict checking. I would love to see something more realworld in place. Default jslint isn't "right" imo (wth you mean I can't i++?), but the original intent with the rhino strict was iirc simply to catch stray commas and glaring syntax errors. I'm attaching my jsl.conf that sits in trunk/ root.

I just jsl --conf jsl.conf and look through the actual warnings. Attaching a copy of that too. It currently includes stuff like:

/Users/dante/Sites/dojo/trunk/dojox/grid/_Scroller.js(79): lint warning: missing break statement
				default: break;
................................^

which is clearly a lie.

Changed 9 years ago by dante

Attachment: jsl.conf added

comment:4 Changed 9 years ago by dante

also i lied about attaching the output. it is > 1meg. run it yourself.

comment:5 Changed 9 years ago by ben hockey

i like the idea of having the config in each individual file. it clearly shows to anyone what our standards are. also, i use aptana as an eclipse plugin and it will lint your code as you're developing and will respect the config in the file if it finds it - although, i had to hack into some of their jars to update to a more recent version of jslint. i expect that there's probably other editors/IDEs which do the same checks for you.

at work, we use a rather strict config, something like the following:

/* JSLint configuration comments */
/*global
	dojo: false,
	dijit: false,
	dojox: false
 */
/*jslint
	indent: 4,
	maxlen: 100,
	nomen: false,
	newcap: true,
	onevar: true,
	white: true,
	plusplus: false
 */

of course, for developing dojo, you would set the globals to true to indicate that they could be modified and the style this enforces is not quite in line with the way a lot of dojo code is written (although afaik it doesn't break the style guide) but it makes for fairly consistent code (including whitespace) from all developers. it also helps that there's a 'referee' who says what's right and wrong - all egos except crockford's ;) are removed from the equation and everybody gets their feelings hurt equally.

in line with what james says, this is probably something to consider for 2.0

comment:6 Changed 9 years ago by Adam Peller

jslint is not under CLA and is incompatible with Dojo's license, which permits doing evil.

comment:7 Changed 9 years ago by James Burke

peller: we do not have to distribute JSLint with the toolkit, just use it on the server for validation, encourage developers to run it, maybe even as a kind of local precommit hook (if that is possible in svn).

comment:8 Changed 9 years ago by Adam Peller

Milestone: tbdfuture

comment:9 Changed 8 years ago by Chris Mitchell

Owner: anonymous deleted

comment:10 Changed 7 years ago by dylan

Resolution: fixed
Status: newclosed

2.x will presumably use JSHint. Closing this out.

comment:11 Changed 7 years ago by bill

Milestone: future2.0
Note: See TracTickets for help on using tickets.