-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
tools: add JSDoc ESLint plugin #38279
Conversation
targos
commented
Apr 18, 2021
- tools: install eslint-plugin-jsdoc
- tools: enable JSDoc ESLint plugin
Start with a single rule (check-tag-names) and configure it to limit the number of required fixes.
/cc @bmeck |
@nodejs/testing |
I wonder how far we are from actually running (fairly permissive) tsc to check these instead :D |
We are very far from that I think :D |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rubber-stamp LTGM although that's a lot of files....
Almost all the files come from the |
@ljharb opinions ^ ? |
I mean, my personal opinion is that vendoring dev deps in general is a waste, and i don’t like jsdoc at all :-) so I’m not sure how helpful I’ll be here. if that eslint plugin could be made to drop lodash tho, that’d address this concern (and also avoid a ton of others, like overly broad CVEs alerting unnecessarily for node). most of lodash is effectively built into the language now. |
I wonder if we could delete unused files… Looking at the code source, it's only using a dozen of
|
The trick is that it’s quite difficult to programmatically determine and remove unused files, and you wouldn’t want to have to do the manual work multiple times. |
I'd be -1 on this for now. I'd like to see the vendored dep made smaller or made explicitly opt in. |