Skip to content
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.

Sign up for GitHub

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] Some name cleanups and formattings #9693

Merged
merged 14 commits into from
Jun 27, 2020
Merged

[KeyVault] Some name cleanups and formattings #9693

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
c0e069c
[KeyVault] Some name cleanups and formattings
sadasant Jun 24, 2020
cfcd18b
package.json types cleanup
sadasant Jun 24, 2020
0b33aab
keyvault-keys node recordings
sadasant Jun 25, 2020
e0eb7cb
keyvault-secrets browser and node recordings
sadasant Jun 25, 2020
9ca3d9a
keyvault-certificates node recordings
sadasant Jun 25, 2020
e8bf2b6
keys recordings
sadasant Jun 26, 2020
32eb969
karma port changes
sadasant Jun 26, 2020
d57495b
cleaning things up
sadasant Jun 26, 2020
678a65d
Merge branch 'master' into keyvault/someCleanup
sadasant Jun 26, 2020
2e26650
keys recordings
sadasant Jun 26, 2020
ba39362
browser recordings for certificates
sadasant Jun 26, 2020
35740e0
missing keys recordings
sadasant Jun 27, 2020
9b29987
fixing keyvault-keys tests by not running the crypto tests in the bro…
sadasant Jun 27, 2020
1cc0352
better comment about local cryptography on the local crypto test
sadasant Jun 27, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
2 changes: 1 addition & 1 deletion sdk/keyvault/keyvault-certificates/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
"node": ">=8.0.0"
},
"files": [
"types/",
"types/keyvault-certificates.d.ts",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not publishing the down leveled types?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this was the feedback by @xirzec , am I wrong here?

(good news for me is that reverting this doesn't require re-recording things)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK nobody can use the non-rolled up types since types for this since we declare the types to be the rollup file in the package.json and it doesn't import from the other files.

So including them doesn't actually do anything except make the package bigger.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh and we don't seem to be using downlevel-dts in this package, so there's no downleveled rollup file.

Copy link
Member

@HarshaNalluru HarshaNalluru Jun 25, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, once you add downlevel-ing you can have something like this
image

For now, the current change is good. 👏

"dist/",
"dist-browser/",
"dist-esm/keyvault-certificates/src",
Expand Down
51 changes: 24 additions & 27 deletions ...tes_client__create_read_update_and_delete/recording_can_abort_creating_a_certificate.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading