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

chore(docs): update ESLint with Prettier, react, jsx-a11y #11783

Closed
wants to merge 13 commits into from
Closed

chore(docs): update ESLint with Prettier, react, jsx-a11y #11783

wants to merge 13 commits into from

Conversation

josefaidt
Copy link
Contributor

@josefaidt josefaidt commented Feb 15, 2019

Description

Updates to ESLint documentation to include:

  • Prettier integration
    • rules from default starter
  • react plugin
    • recommended rule set
    • sample rules
    • link to supported rules
  • jsx-a11y plugin
    • recommended rule set
    • sample rule set
    • link to supported rules
  • write scripts to lint and fix
  • integrate with VS Code
    • includes settings to enable auto-fix

Also updated commands and wording as necessary.

Related Issues

Related to #11778

@josefaidt josefaidt requested a review from a team February 15, 2019 00:07
@josefaidt josefaidt changed the title update ESLint with Prettier, react, jsx-a11y chore(docs) update ESLint with Prettier, react, jsx-a11y Feb 15, 2019
@josefaidt
Copy link
Contributor Author

Per #11778 I will be updating this doc again this evening to include script modification and add instructions for integrating ESLint with VSCode

Copy link
Contributor

@DSchau DSchau left a comment

Choose a reason for hiding this comment

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

I think these are excellent improvements. Using ESLint for real code issues and Prettier for stylistic concerns is a great approach--and we do it with our Gatsby monorepo!

I'm pretty comfortable merging this. @marcysutton want to take a look?

docs/docs/eslint.md Outdated Show resolved Hide resolved
@josefaidt
Copy link
Contributor Author

docs/docs/eslint.md Outdated Show resolved Hide resolved
@wardpeet wardpeet added type: documentation An issue or pull request for improving or updating Gatsby's documentation status: awaiting reviewer response A pull request that is currently awaiting a reviewer's response labels Mar 27, 2019
@wardpeet wardpeet changed the title chore(docs) update ESLint with Prettier, react, jsx-a11y chore(docs): update ESLint with Prettier, react, jsx-a11y Mar 27, 2019
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.

Dang-I just saw this PR made substantial changes to the eslint doc file. My apologies for taking so long on this one, as it is pretty outdated now in comparison. Do you want to take another look to see if there are any changes to be made?

@josefaidt
Copy link
Contributor Author

@marcysutton I don't mind revisiting this, however that PR mentioned changed the entire tone of the doc, and this PR was opened to provide changes that would coincide with what was being discussed in issue #11778

Again, I don't mind revisiting, however I would like to get more information as to how we are wanting to shape the tone of the doc, as well as what is covered (ESLint + Prettier, VSCode integration, etc).

Should I update this doc? Or the new one?

@DSchau
Copy link
Contributor

DSchau commented Apr 15, 2019

@josefaidt I almost feel like this is two different docs at this point.

Setting up ESLint is oftentimes (most of the time?) done separately from Prettier, although there are certainly valid use cases in setting them up at the same time to tap into the power of one or the other.

However - since we want our docs to be approachable to all skill levels and focused on just the purpose of the doc (e.g. ESLint), I think the PR that landed the changes in #12998 is a better fit for this functionality because it configures ESLint with a simple, extensible set up that can be built upon easily.

I do certainly see the point of a prettier + eslint set-up doc, just not sure it should be in the ESlint set up doc if that makes sense! What I would propose is something like:

  • Create a new prettier doc (docs/prettier.md)
  • Focus on the simple case (e.g. adding prettier as a devDep, using an npm script, etc.)
  • Provide paths to learning more (e.g. a section on integrating ESLint + Prettier makes perfect sense)

As such - let's close this out and better focus our efforts on the above suggestions.

Sorry about the crossed wires here and for the lengthy delay process! We very much value the work you did in getting this set-up and apologize that it went un-merged this time! 💜

cc @marcysutton

@DSchau DSchau closed this Apr 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: awaiting reviewer response A pull request that is currently awaiting a reviewer's response type: documentation An issue or pull request for improving or updating Gatsby's documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants