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

fix(docs): update information about eslint-plugin-jsx-a11y functionality #19947

Merged
merged 15 commits into from
Dec 13, 2019

Conversation

madalynrose
Copy link
Contributor

@madalynrose madalynrose commented Dec 4, 2019

Description

update docs/making-your-site-accessible to include information about default settings for eslint-plugin-jsx-a11y and update docs/eslint to clarify how local .eslintrc files are treated in Gatsby

Related PRs

#19946

update making-your-site-accessible to include information about default settings for `eslint-plugin-jsx-a11y`
@madalynrose madalynrose requested a review from a team as a code owner December 4, 2019 21:20
@madalynrose
Copy link
Contributor Author

@marcysutton is it ok to just remove the stuff about customization? It looks like if they implement rule customizations in their own .eslintrc file it will only show warnings and errors in IDE plugins and bypass the console entirely, which gets away from what we're trying to achieve by including all of the rules by default.

@madalynrose madalynrose self-assigned this Dec 4, 2019
@marcysutton
Copy link
Contributor

@madalynrose we can do whatever we want with this, but I'll pose this question: how would a user disable a rule that doesn't apply to their situation, i.e. causing false positives that end up being ignored? Can we update the config documentation to work better with the new changes, so they can customize what goes into the console? I know from my own experience that some of the warnings aren't always relevant.

@madalynrose
Copy link
Contributor Author

re: customizing eslint

having a local .eslintrc overrides all default linting and bypasses Gatsby's eslint-loader by design. I will add a note about this and update our eslint doc to be more clear about this.

Copy link
Contributor

@laurieontech laurieontech left a comment

Choose a reason for hiding this comment

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

This write up is overall super helpful! Added one request for clarification.

docs/docs/making-your-site-accessible.md Outdated Show resolved Hide resolved
docs/docs/eslint.md Outdated Show resolved Hide resolved
Co-Authored-By: LB <barth.laurie@gmail.com>
@bball07
Copy link
Contributor

bball07 commented Dec 11, 2019

This looks great! Awesome job Madalyn! Once this last change gets added this should be good to merge!!

Copy link
Contributor

@marcysutton marcysutton left a comment

Choose a reason for hiding this comment

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

This looks good to me, the only thing I noticed was needing to move a comma!

docs/docs/making-your-site-accessible.md Outdated Show resolved Hide resolved
Co-Authored-By: Marcy Sutton <marcy@gatsbyjs.com>
@laurieontech
Copy link
Contributor

Wahoo! Psyched to get this in.

@laurieontech laurieontech merged commit b7906a8 into master Dec 13, 2019
@delete-merged-branch delete-merged-branch bot deleted the docs-site-a11y-linting-update branch December 13, 2019 14:29
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.

4 participants