search

mozilla-jetpack / jpm

Jetpack Manager for Node.js
https://www.npmjs.org/package/jpm
Mozilla Public License 2.0
164 stars 75 forks source link

Add no-warning-comments rule to .eslintrc.yml? #586

Closed pdehaan closed 8 years ago

pdehaan commented 8 years ago

Steps to reproduce:

  1. Add this to .eslintrc.yml:

    no-warning-comments: warn
  2. $ npm run lint

    Actual results:

$ npm run lint

> jpm@1.1.4 lint /Users/pdehaan/dev/github/jpm
> eslint bin/jpm lib test

/Users/pdehaan/dev/github/jpm/bin/jpm
  20:1  warning  Unexpected 'todo' comment  no-warning-comments

/Users/pdehaan/dev/github/jpm/lib/rdf.js
  249:5  warning  Unexpected 'todo' comment  no-warning-comments

/Users/pdehaan/dev/github/jpm/test/functional/test.run.js
  55:3  warning  Unexpected 'todo' comment  no-warning-comments

/Users/pdehaan/dev/github/jpm/test/unit/test.profile.js
  71:5  warning  Unexpected 'todo' comment  no-warning-comments

✖ 4 problems (0 errors, 4 warnings)

We should probably file bugs for TODOs so they get TODONE.

$ git grep "TODO:"

bin/jpm:// TODO: use .jpmignore file here instead
lib/rdf.js:    // TODO: Figure out why this fails w/ `===`
test/functional/test.run.js:  // TODO: fix this
test/functional/test.run.js:      // TODO: remove this, fix tests!
kumar303 commented 8 years ago

Nothing is ever "done" :)

I'm not sure what this issue is about. Are you asking if we should include the no-warning-comments: warn rule or not? I think it's a good idea as long as people don't mind the 'todo' noise. There will always be todos. We should definitely make sure bugs are filed on them but I also like to see half finished patches integrated early with todo comments (it keeps the project moving faster).

pdehaan commented 8 years ago

Haha, the joke's on you! Apparently I already snuck this in to master in the recent ESLint chaos: .eslintrc.yml:36

So I guess this bug now morphed into "Should we undo what @pdehaan already did?"

🎱

kumar303 commented 8 years ago

ha, yeah, I thought you did! I searched for 5 seconds and couldn't find it though.

I like it. Can we close the issue? I'm not sure what else to do.

pdehaan commented 8 years ago

We can close it, ... or make me revert the no-warning-comments warning.

So let's close the issue, and we can revert later if people find the shameful TODO noise annoying in Travis-CI logs.