-
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
[KeyVault] Common folder #8866
[KeyVault] Common folder #8866
Changes from all commits
7c46f37
ec0f23a
8b82b38
8824a8c
e1f9ea0
960ac88
2efcad1
bce9304
ba15401
996faf4
a3b1ba7
daf036b
e873fcf
29de300
d737798
925bae1
91007b4
bf263c6
4e8265d
963fae8
aae90ff
f423dbe
a56bd99
fea30d9
4de173b
4ae09d5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,15 +20,17 @@ | |
"url": "https://github.com/Azure/azure-sdk-for-js/issues" | ||
}, | ||
"main": "./dist/index.js", | ||
"module": "dist-esm/src/index.js", | ||
"module": "dist-esm/keyvault-certificates/src/index.js", | ||
HarshaNalluru marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"types": "./types/keyvault-certificates.d.ts", | ||
"engines": { | ||
"node": ">=8.0.0" | ||
}, | ||
"files": [ | ||
"types/keyvault-certificates.d.ts", | ||
"types/", | ||
"dist/", | ||
"dist-esm/src", | ||
"dist-browser/", | ||
"dist-esm/keyvault-certificates/src", | ||
"dist-esm/keyvault-common/src", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
"README.md", | ||
"LICENSE" | ||
], | ||
|
@@ -52,8 +54,8 @@ | |
"extract-api": "tsc -p . && api-extractor run --local", | ||
"format": "prettier --write --config ../../.prettierrc.json \"src/**/*.ts\" \"samples/*.ts\" \"test/**/*.ts\" \"*.{js,json}\"", | ||
"integration-test:browser": "karma start --single-run", | ||
"integration-test:node": "nyc mocha -r esm --require source-map-support/register --reporter ../../../common/tools/mocha-multi-reporter.js --timeout 180000 --full-trace dist-esm/test/*.test.js", | ||
"integration-test:node:no-timeout": "nyc mocha -r esm --require source-map-support/register --reporter ../../../common/tools/mocha-multi-reporter.js --no-timeouts --full-trace dist-esm/test/*.test.js", | ||
"integration-test:node": "nyc mocha -r esm --require source-map-support/register --reporter ../../../common/tools/mocha-multi-reporter.js --timeout 180000 --full-trace dist-esm/keyvault-certificates/test/*.test.js", | ||
"integration-test:node:no-timeout": "nyc mocha -r esm --require source-map-support/register --reporter ../../../common/tools/mocha-multi-reporter.js --no-timeouts --full-trace dist-esm/keyvault-certificates/test/*.test.js", | ||
"integration-test": "npm run integration-test:node && npm run integration-test:browser", | ||
"lint:fix": "eslint package.json api-extractor.json src test --ext .ts --fix --fix-type [problem,suggestion]", | ||
"lint": "eslint package.json api-extractor.json src test --ext .ts -f html -o keyvault-certificates-lintReport.html", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
// Copyright (c) Microsoft Corporation. All rights reserved. | ||
// Licensed under the MIT License. See License.txt in the project root for license information. | ||
|
||
export * from "./src"; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
{ | ||
"name": "@azure/keyvault-common", | ||
"sideEffects": false, | ||
"private": true, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wouldn't this library need to ship? If so why it is marked private? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not a library, it's a shared folder that gets built and shipped through the other packages: keyvault-keys, keyvault-secrets and keyvault-certificates. The package.json is there so that VSCode can pick up the dependencies. |
||
"author": "Microsoft Corporation", | ||
"version": "1.0.0", | ||
"license": "MIT", | ||
"description": "Common internal functionality for all of the Azure Key Vault clients in the Azure SDK for JavaScript", | ||
"repository": "github:Azure/azure-sdk-for-js", | ||
"main": "./src/index.ts", | ||
"module": "dist-esm/index.js", | ||
"types": "./types/index.d.ts", | ||
"engines": { | ||
"node": ">=8.0.0" | ||
}, | ||
"scripts": { | ||
"audit": "echo skipped", | ||
"build:minify": "echo skipped", | ||
"build:samples": "echo skipped", | ||
"build:es6": "tsc -p tsconfig.json", | ||
"build:nodebrowser": "echo skipped", | ||
"build:test": "echo skipped", | ||
"build": "npm run extract-api && npm run build:es6 && npm run build:nodebrowser", | ||
"check-format": "prettier --list-different --config ../../.prettierrc.json \"src/**/*.ts\" \"test/**/*.ts\" \"*.{js,json}\"", | ||
"clean": "rimraf dist-esm dist-test typings *.tgz *.log samples/typescript/dist", | ||
"execute:js-samples": "echo skipped", | ||
"execute:ts-samples": "echo skipped", | ||
"execute:samples": "npm run build:samples && npm run execute:js-samples && npm run execute:ts-samples", | ||
"extract-api": "echo skipped", | ||
"format": "prettier --write --config ../../.prettierrc.json \"src/**/*.ts\" \"*.{js,json}\"", | ||
"integration-test:browser": "echo skipped", | ||
"integration-test:node": "echo skipped", | ||
"integration-test:node:no-timeout": "echo skipped", | ||
"integration-test": "npm run integration-test:node && npm run integration-test:browser", | ||
"lint:fix": "eslint package.json src --ext .ts --fix --fix-type [problem,suggestion]", | ||
"lint": "eslint src --ext .ts -f html -o search-lintReport.html || exit 0", | ||
"lint:terminal": "eslint src --ext .ts", | ||
"pack": "npm pack 2>&1", | ||
"prebuild": "npm run clean", | ||
"test:browser": "npm run clean && npm run build:test && npm run unit-test:browser", | ||
"test:node": "npm run clean && npm run build:test && npm run unit-test:node", | ||
"test": "npm run clean && npm run build:test && npm run unit-test", | ||
"unit-test:browser": "echo skipped", | ||
"unit-test:node": "echo skipped", | ||
"unit-test:node:no-timeout": "echo skipped", | ||
"unit-test": "npm run unit-test:node && npm run unit-test:browser" | ||
}, | ||
"dependencies": { | ||
"@azure/core-http": "^1.1.1", | ||
"tslib": "^2.0.0" | ||
}, | ||
"devDependencies": { | ||
"typescript": "~3.9.3" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
// Copyright (c) Microsoft Corporation. All rights reserved. | ||
// Licensed under the MIT License. See License.txt in the project root for license information. | ||
|
||
export * from "./challengeBasedAuthenticationPolicy"; | ||
sadasant marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: this should extend the common one rather than creating more debt ;) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Woah!! Since when??? ;) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be good now! |
||
"extends": "../../../tsconfig.package", | ||
"compilerOptions": { | ||
"declarationDir": "./types", | ||
"outDir": "./dist-esm", | ||
"lib": ["dom"], | ||
"resolveJsonModule": true | ||
}, | ||
"exclude": ["node_modules"], | ||
"include": ["./src/**/*.ts"] | ||
} |
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.
does this mean we have active warnings now?
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 not sure what you mean. This change is needed because the current rules don't work with this project structure. Once this is merged, and after we polish it, I think we will have a good enough idea of how to change the ESLint rules! Let me know what you think.
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 guess I don't see the point in leaving active warnings in a project. Either we should fix the issue (which in this case I think means fixing the rule?) or disable the rule for now and file an issue to follow-up on turning it back on.
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.
(@willmtemple)
Here's the thing:
As we move forward with the test guidelines, we will need to update several things, including the eslint rules. It will take us some time to get there though. I think we would better fix the libraries first, then fix the eslint plugin. What Will did makes sense for our current state. This PR is in an intermediate state.