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

Add meta object documentation for all rules. Closes #48 #79

Merged

Conversation

MarkKragerup
Copy link
Contributor

@MarkKragerup MarkKragerup commented Mar 28, 2022

This pull request is an alternative from #61 and closes issue #48 Difference being:

  • This one has no merge conflicts.
  • This one attempts to do minimal changes to the code beyond adding the meta property for rules.
  • This one uses the current markdown documentation rather than the inline comments as source for the descriptions.
    It still has a lot of indention changes, due to the fact that adding the meta property required converting the export into a one level higher object.
  • Corrects gramatical errors in the documentation.
  • Adds documentation for detect-new-buffer to the readme aswell, as it was missing.

@MarkKragerup
Copy link
Contributor Author

@nzakas I checked the links and made this PR, as an alternative. If you merge it, issue #48 and PR #61 can be closed.

Copy link
Contributor

@nzakas nzakas left a comment

Choose a reason for hiding this comment

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

Thanks for doing this. Some general notes:

  1. When referencing blog posts in the docs, it’s important to indicate what the post is about. I’ve made some suggestions for these.
  2. In the meta info, description should be a single sentence without markdown content.
  3. meta.docs.url should always be a link to the rule documentation online. If no docs are available, this field can be omitted.
  4. It seems like these rules should all be “error” (something bad might be happening here) instead of “suggestion” (an alternate way of doing something).

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
rules/detect-buffer-noassert.js Outdated Show resolved Hide resolved
rules/detect-buffer-noassert.js Outdated Show resolved Hide resolved
MarkKragerup and others added 8 commits March 29, 2022 14:13
Co-authored-by: Nicholas C. Zakas <nicholas@humanwhocodes.com>
Co-authored-by: Nicholas C. Zakas <nicholas@humanwhocodes.com>
Co-authored-by: Nicholas C. Zakas <nicholas@humanwhocodes.com>
@MarkKragerup
Copy link
Contributor Author

@nzakas i corrected all 4 points of feedbacks. The reason i put it as a suggestion instead of an "error" originally was due to the large number of false positives these rules can give. But i have changed it to "error" now, and i think we should rather work on reducing false positives in the rules separately.

Copy link
Contributor

@nzakas nzakas left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks.

@nzakas nzakas merged commit fb1d9ef into eslint-community:main Mar 30, 2022
@lolgab
Copy link

lolgab commented Apr 1, 2022

@nzakas When are you planning to release a new version?
This is a great improvement for everyone who uses ESLint programmatically and reads the meta objects.
It also adds the recommended patterns which are very useful.

@nzakas
Copy link
Contributor

nzakas commented Apr 1, 2022

@lolgab I'd still like to get through the rest of the PRs and issues before releasing a new version.

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.

None yet

3 participants