-
Notifications
You must be signed in to change notification settings - Fork 230
Clean up lint warnings in core/ec* #8409
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
base: master
Are you sure you want to change the base?
Changes from 7 commits
3c09d9a
cd148a2
755d4d1
b3172d0
6b99721
b6493af
595cdaf
081c0b0
196efef
70db524
2a55725
5ae5447
da61e3d
8e11ba7
3dba84f
844e03b
5b640ca
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/ecschema-editing", | ||
| "comment": "", | ||
| "type": "none" | ||
| } | ||
| ], | ||
| "packageName": "@itwin/ecschema-editing" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| { | ||
| "changes": [ | ||
| { | ||
| "packageName": "@itwin/ecschema-locaters", | ||
| "comment": "", | ||
| "type": "none" | ||
| } | ||
| ], | ||
| "packageName": "@itwin/ecschema-locaters" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| { | ||
| "changes": [ | ||
| { | ||
| "packageName": "@itwin/ecschema-metadata", | ||
| "comment": "", | ||
| "type": "none" | ||
| } | ||
| ], | ||
| "packageName": "@itwin/ecschema-metadata" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| { | ||
| "changes": [ | ||
| { | ||
| "packageName": "@itwin/ecsql-common", | ||
| "comment": "", | ||
| "type": "none" | ||
| } | ||
| ], | ||
| "packageName": "@itwin/ecsql-common" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,7 +39,7 @@ export async function addEnumeration(context: SchemaMergeContext, change: Enumer | |
| export async function modifyEnumeration(context: SchemaMergeContext, change: EnumerationDifference, itemKey: SchemaItemKey) { | ||
| const enumeration = await context.targetSchema.lookupItem(itemKey) as MutableEnumeration; | ||
| if(change.difference.type !== undefined) { | ||
| throw new Error(`The Enumeration ${itemKey.name} has an incompatible type. It must be "${primitiveTypeToString(enumeration.type!)}", not "${change.difference.type}".`); | ||
| throw new Error(`The Enumeration ${itemKey.name} has an incompatible type. It must be "${enumeration.type ? primitiveTypeToString(enumeration.type) : "<unknown>"}", not "${change.difference.type}".`); | ||
|
Member
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. Why is the underlying Enumeration._type property possibly undefined? Any valid enumeration MUST have a type set so we should not force callers to test if the type is null.
Contributor
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. @ColinKerr The constructor also has this as optional. It's internal so we can change this. Should we require the parameter or have a default (int or string)? |
||
| } | ||
| if(change.difference.label !== undefined) { | ||
| await context.editor.enumerations.setDisplayLabel(itemKey, change.difference.label); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ | |
| * @module Validation | ||
| */ | ||
|
|
||
| import { expectDefined } from "@itwin/core-bentley"; | ||
| import { | ||
| AnyClass, AnyProperty, CustomAttribute, CustomAttributeContainerProps, ECClass, ECClassModifier, | ||
| ECStringConstants, EntityClass, Enumeration, Mixin, PrimitiveProperty, PrimitiveType, primitiveTypeToString, | ||
|
|
@@ -328,7 +329,7 @@ export async function* incompatibleValueTypePropertyOverride(property: AnyProper | |
| if (!baseType || primitiveType === baseType) | ||
| return; | ||
|
|
||
| return new Diagnostics.IncompatibleValueTypePropertyOverride(property, [property.class.fullName, property.name, baseClass.fullName, primitiveTypeToString(baseType), primitiveTypeToString(primitiveType!)]); | ||
| return new Diagnostics.IncompatibleValueTypePropertyOverride(property, [property.class.fullName, property.name, baseClass.fullName, primitiveTypeToString(baseType), primitiveTypeToString(expectDefined(primitiveType))]); | ||
|
||
| } | ||
|
|
||
| for await (const baseClass of property.class.getAllBaseClasses()) { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
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.
@christophermlawson what is the expected behavior when the item key doesn't resolve to a relationship class?
This should be fixed properly rather than with this hack and the behavior should be documented and tested
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.
@ColinKerr This would be unexpected, so I think an exception would be appropriate here.