#9278 closed enhancement (fixed)
Add a build tool to check for style guide violations
Reported by: | Shane O'Sullivan | Owned by: | Shane O'Sullivan |
---|---|---|---|
Priority: | high | Milestone: | 1.4 |
Component: | BuildTools | Version: | 1.3.0 |
Keywords: | Cc: | ||
Blocked By: | Blocking: |
Description
The Dojo Style Guide is extremely comprehensive, and covers a wide variety of coding standards. While many of these are very difficult to enforce automatically, there are also many style guideline violations that can be detected using a build tool.
I propose adding a new build tool that generates a style guide violation report. A patch is attached which shows an initial approach that could be taken for this.
To use it, apply the patch and then, in util/buildscripts, type:
build action=checkstyle
This generates a report for all of Dojo, Dijit and Dojox, excluding tests and demos folders.
To check just a single folder, type:
build action=checkstyle checkstyleDir=dojo/base
The folder specified is relative to the root of the Dojo folder structure.
Once the report is generated, open the checkstyleReport.html file.
Attachments (5)
Change History (22)
comment:1 Changed 13 years ago by
comment:2 Changed 13 years ago by
Component: | General → BuildTools |
---|
Changed 13 years ago by
Attachment: | checkstyle.3.patch added |
---|
Updated patch that makes it easier to integrate with other code. checkstyleUtil no longer opens and reads a file itself.
Changed 13 years ago by
Attachment: | checkstyle.4.patch added |
---|
Update patch, files moved to their own util/checkstyle folder
Changed 13 years ago by
Attachment: | checkstyle.5.patch added |
---|
Updated patch, support for specifying individual files, throwing exception on failure
comment:3 Changed 13 years ago by
(In [17569]) Refs #9278 Adding a build to to check for style guide violations. It provides a command line interface to run the checks on all JavaScript? files in the Dojo tree, a HTML file to view and manipulate the results, and a PHP file that can be used to save the fixed files.
The readme.txt file contains instructions on its use, and the commmand
checkstyle.sh help
prints more help text.
comment:4 Changed 13 years ago by
comment:5 Changed 13 years ago by
comment:6 Changed 13 years ago by
comment:7 Changed 13 years ago by
comment:8 Changed 13 years ago by
comment:9 Changed 13 years ago by
Huh. I thought we used the "4 spaces per tab" convention. And replacement should be done when certain conditions are met, e.g., proper tab locations, not arbitrary N spaces.
comment:10 Changed 13 years ago by
Eugene,
More accurace fixes are of course welcome. However, I've used these on a few files and they seem to work very well. As I replace four spaces with tabs first, then two spaces, I cover both the case where people have correctly set their editor to use four spaces for tabs, and where they have incorrectly set up their editors to use two spaces per tab.
If you'd like to make some modifications, please feel free to do so.
comment:11 Changed 13 years ago by
comment:12 Changed 13 years ago by
comment:13 Changed 13 years ago by
Status: | new → assigned |
---|
comment:14 Changed 13 years ago by
comment:15 Changed 13 years ago by
comment:16 Changed 13 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Checkstyle tool is now quite stable and usable. Any future issues can be logged as separate tickets, so closing this one.
If we had such a tool, it could be applied as a pre-checkin step also. We've discussed this before, but never gotten far. A simple, but controversial approach would be to just adopt Crockford's jslint rules.