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

Fix ESLint errors and JSDoc comments in linters #5655

Closed
wants to merge 6 commits into from

Conversation

queengooborg
Copy link
Collaborator

This PR fixes up the issues reported by ESLint, as well as updates the JSDoc comments, and performs just a little bit of refactoring for efficiency and consistency in the linter files.

@ghost ghost added the linter 🏡 Issues or pull requests regarding the tests / linter of the JSON files. label Feb 5, 2020
Copy link
Contributor

@ExE-Boss ExE-Boss left a comment

Choose a reason for hiding this comment

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

You shouldn’t be using ESLint to validate JSDoc types, as its parser is woefully out of date.

I recommend using TypeScript’s JSDoc validation instead, as it ensures that the declared types match what’s being used.

test/linter/test-consistency.js Show resolved Hide resolved
test/linter/test-consistency.js Show resolved Hide resolved
test/linter/test-consistency.js Show resolved Hide resolved
test/linter/test-consistency.js Outdated Show resolved Hide resolved
test/linter/test-links.js Show resolved Hide resolved
test/linter/test-links.js Show resolved Hide resolved
test/linter/test-links.js Show resolved Hide resolved
test/linter/test-versions.js Outdated Show resolved Hide resolved
@queengooborg
Copy link
Collaborator Author

FYI @ExE-Boss, I am not using ESLint's built-in JSDoc checks to validate the JSDocs. Instead, I am using a dedicated plugin that is actively maintained (its last release was 13 days ago).

Copy link
Contributor

@ExE-Boss ExE-Boss left a comment

Choose a reason for hiding this comment

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

I forgot that I wanted to mention this last time:

test/linter/test-consistency.js Show resolved Hide resolved
@ddbeck
Copy link
Collaborator

ddbeck commented Mar 25, 2020

This PR appears to partly relate to #5252 and #5635 (comment).

I'm going to suggest closing this PR and breaking it into pieces. It looks like there's a grab bag of changes here: copyright notices (which I'm opposed to generally—I've opened #5879 on this topic), hashbang lines, and JSDoc comments. The ESLint connection seems to be an ESLint plugin that checks for… something that I don't think has been described anywhere. I'd like to be able to review and discuss these separately, since they don't seem to be dependent on each other.

@queengooborg
Copy link
Collaborator Author

queengooborg commented Mar 25, 2020

Sounds good to me, I'll start breaking this down then! I'll mark this as "not ready" and leave it open until the separate PRs have been created.

@queengooborg queengooborg added the not ready ⛔ This is not yet ready to be merged. It's pending a decision, other PR, or a prerequisite action. label Mar 25, 2020
@Elchi3 Elchi3 removed their request for review April 8, 2020 14:08
@queengooborg queengooborg deleted the eslint/linter branch October 31, 2020 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
linter 🏡 Issues or pull requests regarding the tests / linter of the JSON files. not ready ⛔ This is not yet ready to be merged. It's pending a decision, other PR, or a prerequisite action.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants