Skip to content
This repository has been archived by the owner on Aug 26, 2022. It is now read-only.

Nobug : Adds eslint and husky for linting goodness #4199

Merged
merged 2 commits into from Apr 28, 2017

Conversation

schalkneethling
Copy link

Hey @stephaniehobson

This adds eslint to the project. If you use an eslint linter plugin for your editor, you should get real time linting feedback. There is also a lint target with:

npm run lint

I have marked most things as warnings so, the pre-commit hook introduced here will pass for the moment.

.eslintrc.js Outdated
"browser": true,
"es6": true,
"jquery": true,
"jasmine": true
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may not need jasmine here, not sure what MDN uses for JS tests.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't have any JS tests right now. We will probably use jasmine if/when we do to be inline with bedrock but it should be removed for now.

.eslintrc.js Outdated
"gettext": true,
"interpolate": true,
"mdn": true,
"Mozilla": true
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, not sure if we need all of these as globals, added them as they came up but, with some of the exclusions I then introduced, perhaps they are not all needed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also need to add waffle. Nothing jumps out at me from that last as something we can remove if we're not linting the libraries.

Copy link
Contributor

@stephaniehobson stephaniehobson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works great locally.

OMG we have so much work to do 😲

+1 without Jasmine and ES6

.eslintrc.js Outdated
"gettext": true,
"interpolate": true,
"mdn": true,
"Mozilla": true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also need to add waffle. Nothing jumps out at me from that last as something we can remove if we're not linting the libraries.

.eslintrc.js Outdated
module.exports = {
"env": {
"browser": true,
"es6": true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our browser support requirements don't let us use ES6 yet.

@codecov-io
Copy link

Codecov Report

Merging #4199 into master will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4199      +/-   ##
==========================================
- Coverage   86.25%   86.25%   -0.01%     
==========================================
  Files         146      147       +1     
  Lines        9035     9040       +5     
  Branches     1211     1211              
==========================================
+ Hits         7793     7797       +4     
  Misses       1004     1004              
- Partials      238      239       +1
Impacted Files Coverage Δ
kuma/wiki/views/document.py 88.27% <0%> (-0.37%) ⬇️
kuma/settings/common.py 93.33% <0%> (ø) ⬆️
kuma/conftest.py 100% <0%> (ø)
kuma/wiki/jobs.py 95.58% <0%> (+0.35%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 76d590b...8deb3fd. Read the comment docs.

@stephaniehobson stephaniehobson merged commit 80617f5 into master Apr 28, 2017
@stephaniehobson stephaniehobson deleted the add-eslint-to-workflow branch April 28, 2017 17:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants