Automate style checking for javascript files
Horizon currently doesn't have any coding standards for javascript files. We need to have an agreement over one and then automate style checking for js, just like pep8 does for python. It also involves fixing all the inconsistencies in the existing codebase and making changes according to the accepted coding standards.
Blueprint information
- Status:
- Complete
- Approver:
- David Lyle
- Priority:
- Medium
- Drafter:
- Imre Farkas
- Direction:
- Needs approval
- Assignee:
- Radomir Dopieralski
- Definition:
- Approved
- Series goal:
- Accepted for icehouse
- Implementation:
-
Implemented
- Milestone target:
-
2014.1
- Started by
- Imre Farkas
- Completed by
- David Lyle
Related branches
Related bugs
Sprints
Whiteboard
[ifarkas] There are many style guides available. An article [1] by Seravo tries to find a consensus among them, it would make sense for Horizon to adapt it.
As for the automation, integrating a tool like JSHint [2] could do the automatic style checking. We have multiple options:
- similarly to pep8, we can extend the run_tests.sh to run JSHint and print the output as JSHint can be run as a Node.js application
- we can integrate JSHint with the existing qUnit javascript test framework.
I propose the later as it doesn't introduce a dependency to Node.js and has an advantage of running automatically with the selenium tests.
[1] https:/
[2] http://
Gerrit topic: https:/
Addressed by: https:/
JSHint integration
mrunge, Nov 20 2013: since it took us long to get rid of node.js, I wouldn't vote for anything bringing us node.js back, even not through the backdoor.
rdopieralski, 2013-11-25: I'm all for adding style checks for JavaScript, but if we are going to do it, we need to think about the roadmap for bringing the style up to par with the checks first. I don't think that adding the style fixes together with the test check in one huge commit is a feasible approach. Another option is to fix all the style problems in a chain of patches, and rebase the addition of the test at the end of that chain -- but it's going to be a hell merging it with any changes that pop up in js in the mean time. I think that the best approach is to add the jshint to tests with all the failing checks disabled initially, and then fix the issues and enable the checks gradually in subsequent patches. This way we don't have to make one huge change, but at the same time we make sure that the things that we gradually fix remain fixed.
[jpichon, 2013-11-25] This makes sense and would match the way we've been enabling the HACKING tests. ( https:/
Gerrit topic: https:/
Addressed by: https:/
Use === and !== instead of == and != in JavaScript.
Addressed by: https:/
Use [] for new arrays in JavaScript
Addressed by: https:/
Fix confusing use of ! and = in JavaScript
Addressed by: https:/
Fix duplicate definition and scope in JavaScript
Addressed by: https:/
Use dot notation in JavaScript
Addressed by: https:/
Fix duplicate keys in JavaScript
Addressed by: https:/
Fix bad line breaking and radix in JavaScript
Addressed by: https:/
Fix semicolons in JavaScript
Addressed by: https:/
Don't use extra leading zeros in JavaScript
