Add eslint with airbnb/legacy config #61
Conversation
| "eslint-config-airbnb": "^9.0.1", | ||
| "eslint-plugin-import": "^1.7.0", | ||
| "eslint-plugin-jsx-a11y": "^1.2.0", | ||
| "eslint-plugin-react": "^5.1.1" |
There was a problem hiding this comment.
is the react stuff necessary?
There was a problem hiding this comment.
Not strictly necessary, but they are peerDependencies of estlint-config-airbnb, so npm will complain if they are not there 😒 :
- removed mkdirp package, it was unused - formatted/refactored api.js to meet eslint rules - made some minor rule changes to the default airbnb/legacy rules (eslintrc)
| }); | ||
| }); | ||
|
|
||
| return checklist; | ||
| } | ||
|
|
||
| function getSortedItemIds(checklist) { | ||
| var actionable; | ||
|
|
||
| checklist = JSON.parse(JSON.stringify(checklist)); |
There was a problem hiding this comment.
@anthonygarvan does this serve any purpose? it seems like stringifying and parsing should have no effect. Tests still pass when this line is removed completely, which is what I'd expect.
- still needs a light refactor of the getApp method to not shadow variables
|
This should be good to go. All tests are passing. This touched a lot (i.e., all of) the code, so I would appreciate another pair of eyes before merge @anthonygarvan. FYI, the frontend guild today voted to use Airbnb's JavaScript style guide, so this PR will bring Checklistomania up to speed with that: 18F/frontend#98 (comment) |
jseppi
changed the title
|
This is awesome, amazing work!! |
|
@jseppi I merged this, but could you look into why coverage dropped next week? Looks like we are missing some things in the api now. |
|
Sure. Might need a little assistance because I'm not familiar with
|
Ready for review