[Linter] Update linter rule to check if sdk-type exists in package.json#18597
[Linter] Update linter rule to check if sdk-type exists in package.json#18597maorleger merged 8 commits intoAzure:mainfrom
Conversation
bzhang0
requested review from
deyaaeldeen,
maorleger,
mikeharder,
praveenkuttappan,
sadasant and
witemple-msft
as code owners
ghost
added
KeyVault
eslint plugin
customer-reported
|
Thank you for your contribution bzhang0! We will review the pull request and get back to you soon. |
There was a problem hiding this comment.
🚀 this looks fine to me. For the README update I think 3.1.0 is fine, this is the latest version we used when adding ts-no-window and this isn't actually released to npm anymore
Thanks for helping out!
@bzhang0 I can merge this after the README is updated
| if (!["client", "mgmt"].includes(strValue)) { | ||
| context.report({ | ||
| node: node.value, | ||
| message: "sdk-type is not set to `client` or `mgmt`" |
There was a problem hiding this comment.
Perhaps it could be useful to print the unrecognized value too?
| message: "sdk-type is not set to `client` or `mgmt`" | |
| message: `unrecognized sdk-type value: ${strValue}. Expected `client` or `mgmt` instead.` |
Please note the error messages in the tests will need to be updated too.
|
@maorleger @deyaaeldeen I'll have these changes done! |
jeremymeng
left a comment
jeremymeng
left a comment
There was a problem hiding this comment.
Looks good! just one comment
|
|
||
| const strValue = stripPath(value.value); | ||
|
|
||
| if (!["client", "mgmt"].includes(strValue)) { |
There was a problem hiding this comment.
Please add "utility" into the allowed list. It was just added earlier in #18402.
It looks like ts-package-json-sdktype rule entry was duplicated in #18597 - which causes the "number of rules should match the expected value" to report an error. This PR fixes the issue by removing the duplicated entry. I also went ahead and sorted the entries to make these things easier to spot in the future (although there is no guarantee sort will be preserved).
4 participants
This fixes #13214. This PR is an update from a past PR, #18565.
What's new?
ts-package-json-sdktype.tsto enforce the existence ofsdk-typeand that it is eitherclientormgmt. If it does not, then the linter will throw an error.rush lintsrc/configs/index.ts, src/rules/index.ts, andtests/plugin.ts)docs/ruleswhich are now included in the commit!README.mdfile, but I'm not sure what the actual version is (hopefully this isn't too much trouble)Let me know if there's anything I missed!