Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Linting #583

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Linting #583

wants to merge 7 commits into from

Conversation

brettz9
Copy link

@brettz9 brettz9 commented Jul 10, 2020

A large PR as it fixes the linting issues per your current desired config.

Re disabling:

  1. I added a few disable directives such as for the complexity and max-statements rules, though I can undo that if you like.
  2. I also inline-disabled a few cases of no-continue, as for longer code, continue helps prevent overly nested blocks. (Let me know if you prefer to have this rule removed.)
  3. I also disabled no-ternary as I hope you'd reconsider not enforcing this (just restricting nested ternaries), though I can re-enable that and offer fixes if you are set on that.
  4. I wasn't sure if I could change clk_x and clk_y to camel-casing without introducing a breaking change, so I explicitly ignored them in the camel-case rule, but let me know if I can change.
  • Linting: Apply existing rules
  • Linting: Add .eslintignore file (allows linting IDEs to know what should not be linted)
  • Linting: Use overrides for special treatment of config files
  • Linting: Remove no-undefined rule (not useful in strict mode and with no-restricted-names protecting against shadowing; while useful to use undefined for explicit array return)
  • Linting: Comment out no-invalid-this (not practical for jQuery)
  • Linting: Comment out no-ternary (really want to disable?)
  • Linting: Allow clk_x and clk_y non-camel-cased
  • Linting (HTML): Add self-closing tag for non-void <option>; add DOCTYPE; consistent/fixed quotes
  • Docs: Remove stray quote in CONTRIBUTING (tripping up syntax highlighting in Atom)
  • Refactoring/Linting: Throw Error object in place of "abort" string
  • Refactoring: Prefer Array.isArray, move let/const closer to scope
  • npm: Add lint and test script
  • npm: Add eslint devDep.
  • npm: - Bump engines to Node 8 (for Object.values); bump jquery dep. to earliest npm version 1.9.1 (though package.json started in 1.8.0)

If you need a build step so as to convert const to var and for..of for older browsers, I can look to add Babel to the build steps, though in so doing, and if it's all right, I'd like to:

  1. Switch from grunt to package.json scripts only
  2. Switch from PhantomJS to Cypress (which supports Chrome/Edge/Electron and Firefox) can also give code coverage).
  3. If you want a build step, I'd think we could switch source to ESM in the process, and I can offer a build for that by Rollup if you are open to it.
  4. (I also added Object.values, so for older browsers, that may need a polyfill, depending on what you wanted to do about such, e.g., to refer users to core-js if they need these.)

Other items I might tackle if you're ok with it:

  1. If you want much tighter ESLint checks (while accommodating your current stylistic preferences), I might propose my own eslint-config-ash-nazg for a very robust config (drawing together several plugins in the community).
  2. I'd also like to use husky, possibly with lint-staged to avoid the need for handling installation of shell scripts in the repo (keeping it all nicely Node or package.json command driven).

I'm on a very slow connection atm, so not able to download PhantomJS to confirm tests are still passing, but I think if you find any issues, there really shouldn't be many (or I can explain).

- Linting: Add `.eslintignore` file
- Linting: Use `overrides` for special treatment of config files
- Linting: Remove `no-undefined` rule (not useful in strict mode and with `no-restricted-names` protecting against shadowing; while useful to use `undefined` for explicit array return)
- Linting: Comment out `no-invalid-this` (not practical for jQuery)
- Linting: Comment out `no-ternary` (really want to disable?)
- Linting: Allow `clk_x` and `clk_y` non-camel-cased
- Linting (HTML): Add self-closing tag for non-void `<option>`; add DOCTYPE; consistent/fixed quotes
- Docs: Remove stray quote in CONTRIBUTING (tripping up syntax highlighting in Atom)
- Refactoring/Linting: Throw `Error` object in place of "abort" string
- npm: Add `lint `and `test` script
- npm: Add `eslint` devDep.
- Refactoring: Avoid further `let`, bringing declaration closer to scope
@brettz9
Copy link
Author

brettz9 commented Aug 27, 2020

Any chance you'll be able to take a look?

@brettz9
Copy link
Author

brettz9 commented Oct 3, 2020

Hi--I'd like to possibly make some more substantial contributions, but figure it is safer to build on a well-linted foundation.

…dep. to earliest npm version 1.9.1 (though `package.json` started in 1.8.0)
@mgaert
Copy link

mgaert commented Jan 20, 2021

Is it possible to integrate this pull request? because it is hard to work on JQuery3 stuff if it is not lint error free!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants