Skip to content

Conversation

artemruts
Copy link
Contributor

@artemruts artemruts commented Nov 23, 2020

Description

Adds ESLint accessibility plugin so it's easy to do the right thing.

@quinnkeast @felixfbecker do we keep recommended rules? We need to be careful introducing this, all of the default rules are error and may block CI/CD.

Right now our codebase has ~111 a11y errors (yarn all:eslint | grep jsx-a11y | wc -l).

Edit:

It's pretty cool we're running this config against the codebase in CI 👍 ❤️

@felixfbecker
Copy link
Contributor

Since there are so many violations, we would need a PR ready that fixes the violations in the main codebase before merging this one.

Alternatively, something we often do is add rules as warnings, then promote them to errors once every instance is fixed. However in reality often they stay warnings forever, so if possible there can be value in fixing violations in one swoop. But warnings are still better than no warnings :)

@artemruts
Copy link
Contributor Author

Since there are so many violations, we would need a PR ready that fixes the violations in the main codebase before merging this one.

Alternatively, something we often do is add rules as warnings, then promote them to errors once every instance is fixed. However in reality often they stay warnings forever, so if possible there can be value in fixing violations in one swoop. But warnings are still better than no warnings :)

Yeah, I agree that if we introduce them as warnings it'll take a long time. I'll just try to fix them and test very page where I can 👍

@umpox umpox merged commit 5e5a7ea into master Mar 25, 2021
@umpox umpox deleted the adds-eslint-a11y branch March 25, 2021 11:19
@sourcegraph-buildkite
Copy link
Collaborator

🎉 This PR is included in version 0.23.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

4 participants