-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[Cosmos] Fix linting issues #13039
Merged
deyaaeldeen
merged 18 commits into
Azure:master
from
deyaaeldeen:deyaaeldeen/lint-cosmos
Feb 26, 2021
Merged
[Cosmos] Fix linting issues #13039
deyaaeldeen
merged 18 commits into
Azure:master
from
deyaaeldeen:deyaaeldeen/lint-cosmos
Feb 26, 2021
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
zfoster
approved these changes
Jan 5, 2021
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.
lgtm, a lot of changes but we can test with a dev release
this seems to have an actual compilation failure now with the hashing "v2Key" not assignable to boolean |
This was referenced Feb 26, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes linting issues in
src
,samples
,package.json
, andapi-extractor.json
. Fixes #10776.Things to note
@azure/azure-sdk/ts-naming-options
and@azure/azure-sdk/ts-apisurface-supportcancellation
require breaking changes, so I changed their reporting mode towarn
for now.tsdoc/syntax
rule requires the documentation to conform to TSDoc standard. I changed its reporting mode towarn
for now. There is already an issue opened for this: [Cosmos] Migrate documentation comments to TSDoc #12951.@azure/azure-sdk/ts-doc-internal
is looking for the JSDoc tag@ignore
which is no longer a supported behavior. I fixed this here: [ESLint Plugin] ts-doc-internal checks for hidden instead of ignore #13038 and changed its reporting mode towarn
for now. Once [ESLint Plugin] ts-doc-internal checks for hidden instead of ignore #13038 is merged, we will need to revert that change back.@azure/azure-sdk/ts-package-json-module
requires changes to the typescript settings. I think it is better addressed by the cosmos team and I changed its reporting mode towarn
.any
in argument position. My approach to fix this is to try and figure out a concrete type for that argument and I was able to do that in a few places. When I could not, I replacedany
withunknown
.ban-types
prohibits using types such asobject
and{}
. I tried my best to address those, mostly by replacing them usingRecord<string, unknown>
(the type of any object), andunknown
. Feel free to suggest better alternatives in those cases.sdk/cosmosdb/cosmos/src/request/StatusCodes.ts
has a type for status codes computed from constants. However, TypeScript reduces that type tonumber
. So for readability, I defined that type to benumber
.After rebasing changes
import/no-extraneous-dependencies
because it currently fails with this issue: v2.21 recursion error atimport
statement import-js/eslint-plugin-import#1816.dev-tool
's standard rollup config.checkURL
method into its own module so that if in the future another part needs to importURL
, they can do so without importingcheckURL
. Also got rid of therequire
call there because AFAIKnode v8.17
has aurl
module and this should be the version to support at this point. Text Analytics does the same here: https://github.com/Azure/azure-sdk-for-js/blob/master/sdk/textanalytics/ai-text-analytics/src/utils/url.ts.isomorphic-webcrypto
.