chore: Add ESLint and CI #49
Conversation
| "ecmaVersion": 6 | ||
| }, | ||
| "rules": { | ||
| "no-unused-vars": "off", |
There was a problem hiding this comment.
These should also be fixed, but suppressed them to minimize the diff
There was a problem hiding this comment.
As mentioned in w3c/aria#1934, this will require some commits to various ARIA repos to align with the new lint constraints.
| insert_final_newline = true | ||
|
|
||
| [{package*.json,lerna.json}] | ||
| indent_size = 2 |
There was a problem hiding this comment.
Why multiple indent sizes? I generally prefer 2, but am mainly just curious about this 4/2 special case.
There was a problem hiding this comment.
NPM defaults to using 2 spaces, so this ignores it to keep it consistent
There was a problem hiding this comment.
I looked and I think most of the other ARIA specs use 2 (except where there are errors, obvs)
- aria/index.html
- core-aam/index.html
- graphics-aam/index.html
- others
The CSS and JS files are less consistent, but on quick review, I think more (?) of the JS files use 2 spaces as well, even several of the JS files in this diff.
I'll defer to the Chairs and editors (@spectranaut, @pkra, @jnurthen) but my preference would be to change as little as possible in the source, and therefore keep the default spacing to 2 spaces per indent.
No, this won't affect any downstream repos or require changes outside of the existing sync. Other repo that already have linting enabled, already ignore the |
|
We discussed this PR in yesterday's aria editors meeting. I have made an alternative proposal in #94 |
|
The other PR is only for Prettier, which only does formatting. ESLint is about code correctness, and I've just included the prettier and it's plugin in this one to deal with the format/style related rules |
Correct. Another difference is that the other PR makes the changes. |
|
Yeah, I don't see it as alternate, but something in addition to this. You'll probably need to do a full run on the repo first, and decide if you want to ignore files like HTML, which it doesn't handle as nicely |
|
Sorry for being hasty and unclear. My PR is meant to exhibit one way to address the problem discussed for the specs, in particular consistent HTML formatting. The specs don't really have JS and we are trying to remove most of what's here so it's a different problem, really. Having to fix linting errors is annoying for authors; having CI fix things is easier in my experience. (Of course there's some connection since aria-common rolls out to the specs.) I personally don't see a large benefit from adding ESLint checks in aria-common. If respec works with what's here, that seems good enough to me. We are trying to reduce the JS code in this repository as much as we can anyway, making correctness even less of a problem in my eyes. I also admit that maintaining / discussing editor config and linting rules (and pleasing those rules) is not my favorite thing in the world. I'm happy to be overruled by the other editors of course. |
|
I'm fine if this gets closed and the repo is retired, but formatting =/= linting. What isn't fixed in this one is the issue around unused, reused, and undeclared variables |
|
As per last editors meeting, we want to merge this here and we want to test automatic formatting in other repositories. |
Took the basic setup as used in aria-practices now