-
Notifications
You must be signed in to change notification settings - Fork 8
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
chore: Add feat/jsdoc-require-returns-type
eslint rule
#115
base: develop
Are you sure you want to change the base?
chore: Add feat/jsdoc-require-returns-type
eslint rule
#115
Conversation
|
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.
I'm approving based on the CI passing.
I still think it's counterintuitive to be doing ESLint types before we fix/enforce our TS types 🤷
Pinging @ayushnirwal in case he has something to add.
feat/jsdoc-require-returns-type
eslint rulefeat/jsdoc-require-returns-type
eslint rule
(its a |
(better to enforce return types vs TS - on hold) |
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.
Use require-returns to enforce descriptions. (as a warning)
Use explicit-function-return-type to enforce return types in TS (as an error)
Hi @ayushnirwal, cc: @justlevine |
.eslintrc.cjs
Outdated
@@ -71,6 +71,7 @@ module.exports = { | |||
|
|||
// Turn of JSdoc types and use TypeScript types instead. | |||
'jsdoc/no-types': [ 'off' ], | |||
'jsdoc/require-returns-type': [ 'warn' ], |
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.
'jsdoc/require-returns-type': [ 'warn' ], | |
'jsdoc/require-returns': [ 'warn' ], |
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.
@ayushnirwal So, should I also revert the types added to return?
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.
Yes, you can remove types from @returns
tag in JSDOC
…maankhan/snapwp; branch 'develop' of github.com:rtCamp/snapwp into feat/jsdoc-require-returns-type
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.
No need for me to review this again. Once @ayushnirwal approves this and a changeset is added, lmk and I'll merge.
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.
This PR is mergable after this and merge conflicts are resolved. Also, changeset too.
What
Related Issue(s):
Testing Instructions
npm install
&&npm run lint
.Additional Info
eslint
errors.Checklist
npm run changeset
.