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

[added] react/ prefix to rule names #633

Merged
merged 2 commits into from
Jun 14, 2016

Conversation

appsforartists
Copy link
Contributor

The README didn't make it clear that rule names should be prefixed with react/, so I fixed it. Reasonable solutions:

  1. Show an example rules block.
  2. Add react/ to each rule name in the index.

I've done both here. Feel free to choose one or accept both. 😃

The README didn't make it clear that rule names should be prefixed with `react/`, so I fixed it.  Reasonable solutions:

1. Show an example `rules` block.
2. Add `react/` to each rule name in the index.

I've done both here.  Feel free to choose one or accept both.  😃
The plugin has a [recommended configuration](#user-content-recommended-configuration) that enforces React good practices.
```json
"rules": {
"react/jsx-uses-react": "warn",
Copy link
Member

Choose a reason for hiding this comment

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

if we're going to use an example, let's use "error" instead of "warn" - warnings are often ignored and thus mostly useless :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

Honestly, I wasn't sure what the right word was to use there. jsx-uses-react fixes eslint errors, rather than introducing them.

@ljharb
Copy link
Member

ljharb commented Jun 14, 2016

I think I like this change - it's a bit noisier but also more explicit.

I'll let @yannickcr merge it, but it'd be good if @appsforartists wouldn't mind rebasing down to a single commit first? :-)

@appsforartists
Copy link
Contributor Author

@ljharb Isn't that what the squash and merge button is for? 😉

I've been lazy and just made these changes from the GitHub UI.

@ljharb
Copy link
Member

ljharb commented Jun 14, 2016

It's what the "rebase and merge" button should be for :-p blindly squashing commits on merge messes with the changelog :-( but no worries if you're editing in the UI.

@appsforartists
Copy link
Contributor Author

Oh. I thought they fixed that. 😢

@yannickcr yannickcr merged commit 2508576 into jsx-eslint:master Jun 14, 2016
@yannickcr
Copy link
Member

Thanks. It will really help those who are new to ESLint plugins.

@appsforartists
Copy link
Contributor Author

appsforartists commented Jun 14, 2016 via email

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

Successfully merging this pull request may close these issues.

None yet

3 participants