-
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
[core-http] Create private package for core-http 2.0 work #8621
Conversation
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.
Very cool, I'm excited to see this work kickstarted!!!
sdk/core/core-https/README.md
Outdated
|
||
## Next steps | ||
|
||
- Build this library (`core-http`). For more information on how to build project in this repo, please refer to the [Contributing Guide](https://github.com/Azure/azure-sdk-for-js/blob/master/CONTRIBUTING.md). |
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.
nit: Maybe this can be updated to core-https
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.
Updated this with some better placeholders since I had merely copy-pasted the core-http readme before.
"request": "launch", | ||
"name": "Launch Program", | ||
"preLaunchTask": "Build Node", | ||
"program": "${workspaceFolder}\\src\\index.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.
What would it do when launching index.ts? do we want ${file}
or something else to run a sample or test?
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 going to replace these with some better ones.
sdk/core/core-https/README.md
Outdated
|
||
## Key concepts | ||
|
||
You can find an explanation of how this repository's code works by going to our [architecture overview](https://github.com/Azure/azure-sdk-for-js/blob/master/sdk/core/core-http/docs/architectureOverview.md). |
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.
are we going to have a new architecture overview instead of linking to the old one?
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.
Yeah, I'll put a TODO
"dtsRollup": { | ||
"enabled": true, | ||
"untrimmedFilePath": "", | ||
"publicTrimmedFilePath": "./types/coreHttps.d.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.
nit: for future downlevel dts work
"publicTrimmedFilePath": "./types/coreHttps.d.ts" | |
"publicTrimmedFilePath": "./types/latest/coreHttps.d.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.
not really needed, could be done in future.
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.
no time like the present. I also fixed up the build task to run downlevel-dts
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.
Looks good! I'm not sure about core-https as an actual package name that gets released, but it's fine as a cheeky code name we keep internal.
Agreed. We can find a better name or just pave over core-http on release. |
/check-enforcer reset |
/check-enforcer evaluate |
In order to facilitate the refactor work for core-http, I am creating a new private package we can use to merge in refactored code and test it out with autorest/etc.
Once it is fully mature we can decide if we want it to replace the existing core-http folder or not.
For the package name I picked
core-https
to reflect conversations I've had with @bterlson about us not supporting any non-encrypted communication with our services. It's a somewhat cheeky name and I am open to alternative temporary names if folks would prefer something else. :)Refactor work is tracked by Epic #8535