chore: added jsdoc/check-tag-names to ESLint config #65660
Conversation
JoshuaKGoldberg
force-pushed
the
check-tag-names-in-config
branch
from
0708693
to
cef47d5
Compare
|
@JoshuaKGoldberg Thank you for submitting this PR! This is a live comment which I will keep updated. This PR touches some part of DefinitelyTyped infrastructure, so a DT maintainer will need to review it. This is rare — did you mean to do this? 8 packages in this PR (and infra files)
Code ReviewsBecause this is a widely-used package, a DT maintainer will need to review it before it can be merged. You can test the changes of this PR in the Playground. Status
All of the items on the list are green. To merge, you need to post a comment including the string "Ready to merge" to bring in your changes. Diagnostic Information: What the bot saw about this PR{
"type": "info",
"now": "-",
"pr_number": 65660,
"author": "JoshuaKGoldberg",
"headCommitOid": "30799962864493f80931672691c03eedac8d9d3c",
"mergeBaseOid": "748cf8849b6c67026557632fc1fe2ebbbef0700c",
"lastPushDate": "2023-06-07T21:56:23.000Z",
"lastActivityDate": "2023-06-12T18:17:09.000Z",
"mergeOfferDate": "2023-06-12T18:14:54.000Z",
"mergeRequestDate": "2023-06-12T18:17:09.000Z",
"mergeRequestUser": "JoshuaKGoldberg",
"hasMergeConflict": false,
"isFirstContribution": false,
"tooManyFiles": false,
"hugeChange": false,
"popularityLevel": "Critical",
"pkgInfo": [
{
"name": null,
"kind": "edit",
"files": [
{
"path": ".eslintrc.cjs",
"kind": "infrastructure"
},
{
"path": ".eslintrc.json",
"kind": "infrastructure"
},
{
"path": "package.json",
"kind": "infrastructure"
},
{
"path": "tsconfig.json",
"kind": "infrastructure"
}
],
"owners": [],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Critical"
},
{
"name": "ember",
"kind": "edit",
"files": [
{
"path": "types/ember/test/debug.ts",
"kind": "test"
},
{
"path": "types/ember/v3/test/debug.ts",
"kind": "test"
}
],
"owners": [
"chriskrycho",
"jamescdavis",
"wagenet",
"dfreeman"
],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Popular"
},
{
"name": "ember__debug",
"kind": "edit",
"files": [
{
"path": "types/ember__debug/ember__debug-tests.ts",
"kind": "test"
},
{
"path": "types/ember__debug/v3/ember__debug-tests.ts",
"kind": "test"
}
],
"owners": [
"chriskrycho",
"dfreeman",
"jamescdavis",
"wagenet"
],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Popular"
},
{
"name": "gulp-help-doc",
"kind": "edit",
"files": [
{
"path": "types/gulp-help-doc/gulp-help-doc-tests.ts",
"kind": "test"
}
],
"owners": [
"Mikhus"
],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Well-liked by everyone"
},
{
"name": "openfin",
"kind": "edit",
"files": [
{
"path": "types/openfin/.eslintrc.json",
"kind": "package-meta",
"suspect": "edited"
}
],
"owners": [
"chrisbarker",
"rdepena",
"whyn07m3",
"licui3936",
"tomer-openfin"
],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Well-liked by everyone"
},
{
"name": "openui5",
"kind": "edit",
"files": [
{
"path": "types/openui5/sap.f.d.ts",
"kind": "definition"
},
{
"path": "types/openui5/sap.ui.mdc.d.ts",
"kind": "definition"
}
],
"owners": [
"openui5bot",
"petermuessig",
"codeworrior",
"akudev"
],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Well-liked by everyone"
},
{
"name": "react-native",
"kind": "edit",
"files": [
{
"path": "types/react-native/v0.71/Libraries/ActionSheetIOS/ActionSheetIOS.d.ts",
"kind": "definition"
},
{
"path": "types/react-native/v0.71/Libraries/ReactNative/RootTag.d.ts",
"kind": "definition"
},
{
"path": "types/react-native/v0.71/Libraries/TurboModule/RCTExport.d.ts",
"kind": "definition"
}
],
"owners": [
"alloy",
"huhuanming",
"iRoachie",
"timwangdev",
"kamal",
"alexdunne",
"swissmanu",
"bm-software",
"mvdam",
"esemesek",
"mrnickel",
"souvik-ghosh",
"nossbigg",
"saranshkataria",
"tykus160",
"jakebloom",
"ceyhun",
"mcmar",
"theohdv",
"romain-faust",
"bebebebebe",
"Naturalclar",
"chinesedfan",
"vtolochk",
"SychevSP",
"sasurau4",
"256hz",
"doumart",
"drmas",
"jeremybarbet",
"ds300",
"natsathorn",
"connectdotz",
"alexeymolchan",
"alexbrazier",
"kuasha420",
"phvillegas",
"eps1lon",
"ZihanChen-MSFT",
"kelset",
"MateWW",
"lunaleaps",
"saadnajmi"
],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Critical"
},
{
"name": "react-reconciler",
"kind": "edit",
"files": [
{
"path": "types/react-reconciler/test/ReactFiberHostConfigWithNoTestSelectors.ts",
"kind": "test"
}
],
"owners": [
"Methuselah96",
"zhanghaocong",
"mathieudutour"
],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Popular"
},
{
"name": "subset-font",
"kind": "edit",
"files": [
{
"path": "types/subset-font/subset-font-tests.ts",
"kind": "test"
}
],
"owners": [
"omacranger"
],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Well-liked by everyone"
}
],
"reviews": [
{
"type": "approved",
"reviewer": "sheetalkamat",
"date": "2023-06-12T18:14:07.000Z",
"isMaintainer": true
},
{
"type": "stale",
"reviewer": "sandersn",
"date": "2023-06-01T20:22:59.000Z",
"abbrOid": "18e3302"
}
],
"mainBotCommentID": 1572246033,
"ciResult": "pass"
} |
|
🔔 @JoshuaKGoldberg — there are no owners, but it would still be good if you find someone to review this PR in the next few days, otherwise a maintainer will look at it. (And if you do find someone, maybe even recruit them to be a second owner to make future changes easier...) |
typescript-bot
moved this from Needs Author Action
to Needs Maintainer Action
in Old Pull Request Status Board
| { | ||
| // TODO: Some (but not all) of these tags should likely be removed from this list. | ||
| // Additionally, some may need to be contributed to eslint-plugin-jsdoc. | ||
| definedTags: [ |
There was a problem hiding this comment.
is this a list of tags that are used on DT and explicitly allowed, or is it the opposite: tags that are explicitly disallowed (and used to be on DT)
There was a problem hiding this comment.
It's the tags that are used on DT and explicitly allowed. https://github.com/gajus/eslint-plugin-jsdoc/blob/main/docs/rules/check-tag-names.md#definedtags
|
Here are the 21 packages with failures for a local run on all packages: The ember packages all have a tag |
typescript-bot
moved this from Needs Maintainer Action
to Needs Author Action
in Old Pull Request Status Board
|
Inspecting the JavaScript source for this package found some properties that are not in the .d.ts files. react-reconciler (unpkg)was missing the following properties:
as well as these 30 other properties...createComponentSelector, createContainer, createHasPseudoClassSelector, createHydrationContainer, createPortal, createRoleSelector, createTestNameSelector, createTextSelector, deferredUpdates, discreteUpdates, findAllNodes, findBoundingRects, findHostInstance, findHostInstanceWithNoPortals, findHostInstanceWithWarning, flushControlled, flushPassiveEffects, flushSync, focusWithin, getCurrentUpdatePriority, getFindAllNodesFailureDescription, getPublicRootInstance, injectIntoDevTools, isAlreadyRendering, observeVisibleRects, registerMutableSourceForHydration, runWithPriority, shouldError, shouldSuspend, updateContainer |
typescript-bot
added
Critical package
Edits multiple packages
Too Many Owners
Check Config
| @@ -11,7 +11,7 @@ const { | |||
| const assert: typeof Ember.assert = Ember.assert; | |||
|
|
|||
| /** | |||
| * @ember/debug tests | |||
| * `@ember/debug` tests | |||
There was a problem hiding this comment.
Pending gajus/eslint-plugin-jsdoc#1116, this seemed like the most straightforward resolution.
typescript-bot
moved this from Needs Author Action
to Needs Maintainer Action
in Old Pull Request Status Board
People who would have been pingedchriskrycho jamescdavis wagenet dfreeman Mikhus chrisbarker rdepena whyn07m3 licui3936 tomer-openfin openui5bot petermuessig codeworrior akudev alloy huhuanming iRoachie timwangdev kamal alexdunne swissmanu bm-software mvdam esemesek mrnickel souvik-ghosh nossbigg saranshkataria tykus160 jakebloom ceyhun mcmar theohdv romain-faust bebebebebe Naturalclar chinesedfan vtolochk SychevSP sasurau4 256hz doumart drmas jeremybarbet ds300 natsathorn connectdotz alexeymolchan alexbrazier kuasha420 phvillegas eps1lon ZihanChen-MSFT kelset MateWW lunaleaps saadnajmi Methuselah96 zhanghaocong mathieudutour omacranger |
|
@sandersn Thank you for reviewing this PR! The author has pushed new commits since your last review. Could you take another look and submit a fresh review? |
|
Note that the test failures in eb565bf are unrelated to this PR. The later commit 3079996 undid changes to those packages. I'd only touched the packages to make sure that they don't have ESLint complaints. |
typescript-bot
added
Maintainer Approved
Self Merge
typescript-bot
moved this from Needs Maintainer Action
to Waiting for Author to Merge
in Old Pull Request Status Board
|
@JoshuaKGoldberg: Everything looks good here. I am ready to merge this PR (at 3079996) on your behalf whenever you think it's ready. If you'd like that to happen, please post a comment saying:
and I'll merge this PR almost instantly. Thanks for helping out! ❤️ |
|
Ready to merge |
typescript-bot
moved this from Waiting for Author to Merge
to Recently Merged
in Old Pull Request Status Board
…o ESLint config by @JoshuaKGoldberg * chore: added jsdoc/check-tag-names to ESLint config * Added a stub tsconfig.json * Temporarily add comment to files to force DT run * Assorted fixes * Switch to backticks * Remove temporary touch tags
Context: Part of part of #648, porting old TSLint rules to ESLint equivalents. This finishes migrating
no-redundant-jsdoc2tojsdoc/check-tag-names.This finishes the work in #65080, #65315, and #65445 by finally checking the rule into the ESLint config. Which I also converted to
.cjsso I could add comments about trimming down that big list of allowed tags.Also adds a stub
tsconfig.jsonsoproject: truealways has a fallback.