788082 - [socorro-crashstats] run linters on jenkins build

Status: RESOLVED FIXED

(Socorro :: Webapp, task)

Description

[Robert Helmer [:rhelmer]]

We should run linters for everything the project uses:

* python (check.py ?)
* html (https://github.com/mozilla/html5-lint ?)
* js (gjslint ?) 
* css ('css lint' ?)

Comment 1

[Schalk Neethling [:espressive]]

For JavaScript I would recommend JSHint. CSS Lint is good but, we will have to tweak the options, especially in the beginning or else we will have LOTS of warning etc.

Comment 2

[[DEACTIVATED] Adrian Gaudebert]

A huge +1 on this, especially the check.py part. Also, do we have clear doc somewhere about git hooks like Lonnen's https://github.com/Lonnen/webtoolstools ?

Comment 3

[Schalk Neethling [:espressive]]

Spoke to rhelmer on IRC a little the other day and a really great option for linting, minifying etc. of the front end assets would be Grutjs along with Nodejs and then we can use JSHint, CCLint, the LESS preprocessor as well as well as the grunt-html plugin.

I have done the basis of this for the sandstone project and it can be easily ported to socorro-crashstats

Comment 4

[Robert Helmer [:rhelmer]]

grunt.js does look good, I tried it with a few linters and it seems to work fine. I guess we don't really need it's way of doing minifcation since we are using django's compressor (we'll need to do this as part of the build though, I will file a separate bug for this).

One thing I realized is that running an HTML linter will not be trivial since our templates are just fragments of HTML. I can easily work around this when running tidy by hand (bug 788063) but running this as a static process at build time will be tricky...

There are some django middleware apps that do HTML validation but we'd need some kind of runtime test (might be doable by jenkins, but might be outside of the scope of this bug, will have to try it and see)

Comment 5

[Peter Bengtsson [:peterbe]]

+1 on jshint. It's much less archaic than gjslint. 

-1 on running tidy on templates. Isn't tidy a tool to correct broken HTML. Why would you want that? Or is it just used to run in dry-run mode to see if the HTML isn't validating.

Comment 6

[Robert Helmer [:rhelmer]]

(In reply to Peter Bengtsson [:peterbe] from comment #5)
> +1 on jshint. It's much less archaic than gjslint. 
> 
> -1 on running tidy on templates. Isn't tidy a tool to correct broken HTML.
> Why would you want that? Or is it just used to run in dry-run mode to see if
> the HTML isn't validating.

Per comment 0 I was suggesting html5-lint (tidy being a one-time thing happening in a separate bug) but per comment 4 it's more complicated than that since these are fragments of HTML documents.

Comment 7

[Lonnen :lonnen]

@adrian - once we settle on the linters, we can modify the hooks, and maybe move them into the socorro repo proper with an install script (it's all relative paths, moving things into .git/)

I'd be happy if we installed the easy linters now and split additional bugs out for more difficult to choose or set up linters.


related but not blocking, I filed bug 789996 to run CI on pull requests. Linting during the build step will be most beneficial once that is set up.

Comment 9

[[github robot]]

Commits pushed to master at https://github.com/mozilla/socorro-crashstats

https://github.com/mozilla/socorro-crashstats/commit/49c6a26647b1cadf516e4d43afcb4fa00f429bbf
bug 788082 - run check.py on build

this adds a linter to our build step, which will force a failed build if
style violations are found.

https://github.com/mozilla/socorro-crashstats/commit/a8ef2ed33fe6fb2aa639adc61f5c979005bc815c
Merge pull request #157 from Lonnen/788082-check-py-on-build

bug 788082 - run check.py on build

Comment 10

[Peter Bengtsson [:peterbe]]

So, what's missing is jshint on jenkins. However, I don't know how to make it NOT block on jshint errors in third-party libs that we add in. Is our naming convention good enough to distinguish our contributions? 

Again, I don't see any point with the html5 linter since we can't run it on our .html files anyway on jenkins. 

Considering now that https://bugzilla.mozilla.org/show_bug.cgi?id=789996 is resolved. Can we resolve this bug too?

Comment 11

[Peter Bengtsson [:peterbe]]

:rhelmer

Can we resolve this bug?

We are not able to run jshint in leeroy because it'll barf of third-party libs that we put next to js code we're personally responsible for. 

And also, we can't run linters on HTML since jenkins does not have a tool to run the site so we can't get the generated HTML output.