Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[Docs] Recategorize rules in readme #3226

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

golopot
Copy link
Contributor

@golopot golopot commented Feb 28, 2022

@codecov-commenter
Copy link

codecov-commenter commented Feb 28, 2022

Codecov Report

Merging #3226 (5d8455f) into master (24bf594) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3226   +/-   ##
=======================================
  Coverage   97.66%   97.66%           
=======================================
  Files         121      121           
  Lines        8519     8519           
  Branches     3075     3075           
=======================================
  Hits         8320     8320           
  Misses        199      199           
Impacted Files Coverage Δ
lib/rules/boolean-prop-naming.js 99.27% <ø> (ø)
lib/rules/destructuring-assignment.js 98.93% <ø> (ø)
lib/rules/function-component-definition.js 98.52% <ø> (ø)
lib/rules/jsx-boolean-value.js 100.00% <ø> (ø)
lib/rules/jsx-filename-extension.js 100.00% <ø> (ø)
lib/rules/jsx-fragments.js 98.93% <ø> (ø)
lib/rules/jsx-pascal-case.js 100.00% <ø> (ø)
lib/rules/no-invalid-html-attribute.js 96.71% <ø> (ø)
lib/rules/no-multi-comp.js 100.00% <ø> (ø)
lib/rules/no-set-state.js 100.00% <ø> (ø)
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 24bf594...5d8455f. Read the comment docs.

@golopot golopot force-pushed the docs-categories branch 2 times, most recently from 2a2d97a to 793d537 Compare March 5, 2022 03:01
Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

It still feels useful to distinguish jsx-specific rules; not everyone uses jsx with react.

Copy link
Contributor Author

@golopot golopot left a comment

Choose a reason for hiding this comment

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

I am thinking that best practice is a subjective word. A rule might be a best practice for me but not for other people. If a rule a considered a good practice for 50% of developers, it might considered a best practice among these developers, and considered a stylistic issue among other developers.

It still feels useful to distinguish jsx-specific rules; not everyone uses jsx with react.

I think the prefix in rule names should suffice this need.

@ljharb
Copy link
Member

ljharb commented Mar 7, 2022

I'm not sure that's true, given that some of the jsx rules cover non-jsx as well.

As for best practice, of course it's subjective, but its perfectly fine to be opinionated. All of the things I've indicated are prevailing community-wide best practices; if someone disagrees, they are free to disable the rule.

@ljharb ljharb force-pushed the master branch 6 times, most recently from 59af733 to 865ed16 Compare November 11, 2022 02:45
@ljharb ljharb force-pushed the master branch 4 times, most recently from 069314a to 181c68f Compare November 18, 2022 17:19
@ljharb ljharb changed the title [Docs]: Recategorize rules in readme [Docs] Recategorize rules in readme Nov 27, 2022
@codecov
Copy link

codecov bot commented Nov 27, 2022

Codecov Report

Merging #3226 (1134494) into master (01ab399) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #3226   +/-   ##
=======================================
  Coverage   97.58%   97.58%           
=======================================
  Files         130      130           
  Lines        9218     9218           
  Branches     3341     3341           
=======================================
  Hits         8995     8995           
  Misses        223      223           
Impacted Files Coverage Δ
lib/rules/boolean-prop-naming.js 99.39% <ø> (ø)
lib/rules/destructuring-assignment.js 97.39% <ø> (ø)
lib/rules/function-component-definition.js 99.00% <ø> (ø)
lib/rules/jsx-boolean-value.js 100.00% <ø> (ø)
lib/rules/jsx-filename-extension.js 100.00% <ø> (ø)
lib/rules/jsx-fragments.js 98.93% <ø> (ø)
lib/rules/jsx-pascal-case.js 100.00% <ø> (ø)
lib/rules/no-multi-comp.js 100.00% <ø> (ø)
lib/rules/no-set-state.js 100.00% <ø> (ø)
lib/rules/no-typos.js 100.00% <ø> (ø)
... and 4 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

# for free to join this conversation on GitHub. Already have an account? # to comment
Development

Successfully merging this pull request may close these issues.

Categorize rules in docs as one of Possible Problems, Suggestions, and Formatting
3 participants