-
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] Use ES2017 #15744
[Core] Use ES2017 #15744
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.
Thanks for doing this! Just one note about core-http
sdk/core/core-http/tsconfig.json
Outdated
@@ -3,7 +3,6 @@ | |||
"compilerOptions": { | |||
"module": "commonjs", | |||
"outDir": "dist", | |||
"target": "es5", |
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 think we have to major bump core-http for this, since ServiceClient
is extended by generated clients (which could be targeting ES5 still, like Storage)
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.
Oh yes that went over my head! I reverted the change there for now and I am not really sure if it is worth doing given that we are migrating everyone off core-http in the next semester.
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.
EDIT: I will not update core-http and core-lro in this PR.
Please ensure the related issue does not get closed until we find a path forward for core-http and core-lro.
* [Core] Use ES2017 * address feedback * revert core-lro change
Related to #15640.
EDIT: I will not update core-http and core-lro in this PR.