Make WordPress Core

Opened 11 years ago

Closed 11 years ago

#25778 closed defect (bug) (invalid)

jshint should throw no errors

Reported by: jorbin's profile jorbin Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: General Keywords:
Focuses: Cc:


With the addition of a jshintrc and the appropriate grunt tasks in r25960, we now can see all sorts of errors. In order for jshint to be of much use, we need the default state to be it running clean.

Change History (2)

#1 @kadamwhite
11 years ago

  • Cc kadamwhite@… added

Some background from the wcbos contributor day: This JSHintRC is based on the jQuery project's jshintrc, per #25187. Of existing styleguides, it was the closest to our preferences for whitespace and documentation rigor, and "how jQuery does it" has been invoked more than any other style guide when JS formatting comes up in trac. You may notice:

  • There's a LOT of errors!
  • Converting == to === (and != to !==) makes up the majority of the potentially functionality-impacting changes
  • {} around single-line if statements make up the majority of the less-likely-to-be-functionality-impacting changes

gnarf37 & I proposed a file-by-file workthrough of the JS in the system; nacin suggested an alternative approach of picking one type of error at a time and working on more broad-based changes. My preference for the file-by-file option was based on the fact that you can thoroughly exercise that file's area of functionality, but I do see Nacin's point (a {} patch would be less risky than a threquals ticket, for example). Worth discussing.

#2 @jorbin
11 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to invalid
  • Status changed from new to closed

During the js standards meeting, we decided that the route to go was not one big ticket, but individual tickets for each file (which may contain multiple patches.)

Note: See TracTickets for help on using tickets.