Added CheckStyle reporter #118

Closed
wants to merge 3 commits into
from

3 participants

@relaxnow

Hi!

I added a CheckStyle reporter that we currently use with Bamboo and the CheckStyle plugin for that.
Would you be willing to integrate it?

Cheers,
Boy

@brentlintner

Thanks for the contribution. Definitely something to add! :-)

I was just about to add this and publish a new release, but when running it on a project (with errors), I got this exception (as a reference, the project with errors is this one).

/Users/brent/src/node-jshint/lib/reporters/checkstyle.js:37
            files[result.file].errors.push({
                                  ^
TypeError: Cannot call method 'push' of undefined
    at /Users/brent/src/node-jshint/lib/reporters/checkstyle.js:37:39
    at Array.forEach (native)
    at module.exports.reporter (/Users/brent/src/node-jshint/lib/reporters/checkstyle.js:29:17)
    at Object.module.exports.hint (/Users/brent/src/node-jshint/lib/hint.js:108:32)
    at Object.module.exports.interpret (/Users/brent/src/node-jshint/lib/cli.js:157:21)
    at Object.<anonymous> (/Users/brent/src/node-jshint/bin/hint:2:25)
    at Module._compile (module.js:449:26)
    at Object.Module._extensions..js (module.js:467:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)

Thoughts? I'm not quire sure it is the reporter (seems results is getting some bad info). Going to look into it more when I have a chance.

Also, there is some lint errors introduced by your commit (./configure && jake lint), but I can also clean those up myself, unless you don't mind cleaning that up and doing a rebase and re-push.

@relaxnow

Oops, sorry, didn't know you adhered to JSLint, I fixed everything I could find.
The failure to resolve .errors is a bug, turns out I fixed this and other bugs after testing locally on our CI machine but forgot to commit (stupid, stupid, stupid), unfortunately I am on vacation right now and I can't access that machine to see what else I fixed, so I retested the reporter and fixed some more bugs. When I get back from vacation I'll check out the local modifications.
Also, I tried rebasing, but the last commit on master is a month ago, so the rebase did nothing?
Is there something I can do (I'm not exactly a git pro yet)?

@brentlintner

Ah, no worries! (whenever you get a chance- looks like you may have already pushed).

As for rebasing, sorry (to have misled), I should of said rebase and squash commits (so there is only one). I usually use git rebase -i master and mark all but one with an s (in the text editor). :-)

@brentlintner

Is this good to review and look to pull in now? Don't worry about the rebasing (or if you don't have time, or whichever). I can even do that myself, or just merge the commits in- Not a big deal, really (just me nitpicking!). :-)

Thanks!

@markstory

Is there anything that needs to be done for this to get merged in? I'd be happy to help get it ready for the next release.

@brentlintner

Looks like everything is in working order. Merged and published as v0.8.1. Thanks again for the contribution. cc/ @markstory @relaxnow

@markstory

Awesome, thanks for merging it in. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment