-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Deprecation for resources #6120
Conversation
@tfili, thanks for the pull request! Maintainers, we have a signed CLA from @tfili, so you can review this at any time.
I am a bot who helps you make Cesium awesome! Contributions to my configuration are welcome. 🌍 🌎 🌏 |
Source/Renderer/loadCubeMap.js
Outdated
@@ -66,6 +68,10 @@ define([ | |||
} | |||
//>>includeEnd('debug'); | |||
|
|||
if (defined(allowCrossOrigin)) { | |||
deprecationWarning('loadCubeMap.allowCrossOrigin', 'The allowCrossOrigin parameter has been deprecated. It no longer needs to be specified.'); |
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.
This function is private, so we shouldn't need to deprecate anything.
@@ -106,6 +107,10 @@ define([ | |||
} | |||
//>>includeEnd('debug'); | |||
|
|||
if (defined(options.headers)) { |
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.
This is private as well, so we shouldn't need this deprecation.
Just those comments, nothing else jumps out at me (and this will get another look in the main PR) so once you address them I'll merge (or you can just merge yourself since this goes into a branch). |
A bunch of places now take a
Resource
as a url. Some of these classes also took other parameters that are now part of the resource (eg. proxy, headers, query). These parameters have been deprecated since they can be passed in as part of the resource instance.