-
Notifications
You must be signed in to change notification settings - Fork 216
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
Remove deprecated 3.x APIs in core-common #7572
base: master
Are you sure you want to change the base?
Changes from all commits
274667f
2b29b19
80e67e0
c0dc76a
6e2f826
1a9a2c8
7036308
582f844
76974ed
3123e2d
15c7a2a
8af7cde
a5de240
c812de2
c7846d9
c18fbba
6df6e5c
8c3ff42
89d1d50
3d1e450
b689236
9dfb919
f85395b
258dca9
f30c5a3
a383930
8a4ea71
3554ce4
e28fff6
1abf021
e7afce9
521bb73
ed08069
5e04b24
2b762ee
3434050
43b5b7f
1bcf102
e68d362
300e9af
04a4639
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
{ | ||
"changes": [ | ||
{ | ||
"packageName": "@itwin/core-backend", | ||
"comment": "Remove 3.x Deprecated APIs", | ||
"type": "none" | ||
} | ||
], | ||
"packageName": "@itwin/core-backend" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
{ | ||
"changes": [ | ||
{ | ||
"packageName": "@itwin/core-common", | ||
"comment": "Remove 3.x Deprecated APIs", | ||
"type": "none" | ||
} | ||
], | ||
"packageName": "@itwin/core-common" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
{ | ||
"changes": [ | ||
{ | ||
"packageName": "@itwin/core-extension", | ||
"comment": "Remove 3.x Deprecated APIs", | ||
"type": "none" | ||
} | ||
], | ||
"packageName": "@itwin/core-extension" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -43,7 +43,9 @@ export namespace RenderMaterial { | |
return Math.max(0.0, Math.min(1.0, value)); | ||
} | ||
|
||
/** @deprecated in 3.x. Use [CreateRenderMaterialArgs]($frontend). */ | ||
/** Params for use in old CreateMaterial functions. Use [CreateRenderMaterialArgs]($frontend) instead. | ||
* @internal | ||
*/ | ||
export class Params { | ||
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. These params are also still in use, I'm going to try swapping them out for CreateRenderMaterialArgs 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. RenderTexture.Params is being using in multiple places in core\frontend I was able to change one of the files, ParseImdlDoucment.ts, but I'm not sure if its implemented correctly as there is some logic around parsing colors. @aruniverse @ben-polinsky Who would be best to talk to for more info on how to proceed. 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. @pmconne will be your best starting point 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. Looks like some of the core-frontend usage is being cleaned up in #7591. Bear in mind that removing deprecated APIs just means making them unavailable to consumers of the public API. While generally we'll want to delete the APIs entirely, if it's too complicated to clean up our internal usage of a particular one, we can instead move it to /src/internal, and export it from cross-package.ts if it's used in other packages within itwinjs-core. 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. grep for the namespace here looks useless now that the only thing exposed is marked internal, wonder if we deprecate the namespace or just mark it as internal too, or honestly remove it entirely and move the Params class into |
||
/** If the material originates from a Material element in the [[IModel]], the Id of that element. */ | ||
public key?: string; | ||
|
@@ -75,7 +77,6 @@ export namespace RenderMaterial { | |
public constructor(key?: string) { this.key = key; } | ||
|
||
/** Obtain an immutable instance of a RenderMaterial with all default properties. */ | ||
// eslint-disable-next-line @typescript-eslint/no-deprecated | ||
public static readonly defaults = new Params(); | ||
|
||
/** A value from 0.0 (fully-transparent) to 1.0 (fully-opaque) controlling the transparency of surfaces to which this material is applied; | ||
|
@@ -87,9 +88,7 @@ export namespace RenderMaterial { | |
} | ||
|
||
/** Create a RenderMaterial params object using specified key and ColorDef values, as well as an optional texture mapping. */ | ||
// eslint-disable-next-line @typescript-eslint/no-deprecated | ||
public static fromColors(key?: string, diffuseColor?: ColorDef, specularColor?: ColorDef, emissiveColor?: ColorDef, reflectColor?: ColorDef, textureMap?: TextureMapping): Params { | ||
// eslint-disable-next-line @typescript-eslint/no-deprecated | ||
const materialParams = new Params(); | ||
materialParams.key = key; | ||
materialParams.diffuseColor = diffuseColor; | ||
|
@@ -102,5 +101,5 @@ export namespace RenderMaterial { | |
} | ||
} | ||
|
||
// eslint-disable-next-line @typescript-eslint/no-deprecated | ||
|
||
Object.freeze(RenderMaterial.Params.defaults); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -75,8 +75,8 @@ export namespace RenderTexture { | |
} | ||
|
||
/** Parameters used to construct a [[RenderTexture]]. | ||
* @deprecated in 3.x. use RenderSystem.createTexture and CreateTextureArgs. | ||
* @public | ||
* Use RenderSystem.createTexture and CreateTextureArgs. | ||
* @internal | ||
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. same comments here as in RenderMaterial.ts |
||
*/ | ||
export class Params { | ||
/** A string uniquely identifying this texture within the context of an [[IModel]]. Typically this is the element Id of the corresponding [Texture]($backend) element. | ||
|
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.
we should move this file into
core/common/src/internal