Skip to content
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

Introduced ProfileSolidInformationRecord classes. #15

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

diegoalexdiaz
Copy link
Contributor

No description provided.

@CLAassistant
Copy link

CLAassistant commented Nov 19, 2021

CLA assistant check
All committers have signed the CLA.

Added IProfiledSolidDefinition mix-in as source end-point for relationships referencing a Profile.
@ColinKerr
Copy link
Member

ColinKerr commented Nov 19, 2021

@diegoalexdiaz have you run the schema inventory job to update the inventory? You will also have to sign the CLA.

@naveedkhan8067 schema validation jobs are being skipped on this branch

@naveedkhan8067
Copy link
Collaborator

@diegoalexdiaz have you run the schema inventory job to update the inventory? You will also have to sign the CLA.

@naveedkhan8067 schema validation jobs are being skipped on this branch

Hey, this seems like a "Draft" PR and right now the validation builds only triggered with PR's which are not "Draft"

@ColinKerr
Copy link
Member

@diegoalexdiaz have you run the schema inventory job to update the inventory? You will also have to sign the CLA.
@naveedkhan8067 schema validation jobs are being skipped on this branch

Hey, this seems like a "Draft" PR and right now the validation builds only triggered with PR's which are not "Draft"

You're right. Sorry about that.

@naveedkhan8067
Copy link
Collaborator

@diegoalexdiaz have you run the schema inventory job to update the inventory? You will also have to sign the CLA.
@naveedkhan8067 schema validation jobs are being skipped on this branch

Hey, this seems like a "Draft" PR and right now the validation builds only triggered with PR's which are not "Draft"

You're right. Sorry about that.

No problem!

Corrected typeName for TransitionType property.
@diegoalexdiaz diegoalexdiaz marked this pull request as ready for review April 6, 2022 19:51
@diegoalexdiaz diegoalexdiaz requested a review from a team as a code owner April 6, 2022 19:51
@diegoalexdiaz diegoalexdiaz changed the title Introduced ProfileSolidDefinition classes. Introduced ProfileSolidInformationRecord classes. Apr 6, 2022
@diegoalexdiaz
Copy link
Contributor Author

Interesting spelling mistake! Do we need Category's name that long? how about: ""ProfiledSolidRecordProperties"

Is your question about just the property-category name? or the usage of "ProfiledSolidInformationRecord" throughout the schema?

If it is the second, we have a decision to make. Class names in BIS schemas tend to replicate the naming introduced in BisCore when there's not a better name already established for a concept in a given domain. The name for the base class from BisCore in this case is "InformationRecord" so those two words have been typically replicated together whenever used.

I share your concern of the long names in the schema, so the decision to be made is about deviating from the naming conventions used everywhere else to achieve shorter class names in this particular case (i.e. ProfiledSolidInformationRecord --> ProfiledSolidRecord).

Any strong opinions @caseymullen, @ColinKerr and others?

Domains/1-Common/Profiles/Profiles.ecschema.xml Outdated Show resolved Hide resolved
@@ -366,4 +367,134 @@
<ECProperty propertyName="Width" typeName="double" description="Extent of the capsule in the direction of the x-axis." category="ProfileProperties" kindOfQuantity="AECU:LENGTH_SHORT" />
<ECProperty propertyName="Depth" typeName="double" description="Extent of the capsule in the direction of the y-axis." category="ProfileProperties" kindOfQuantity="AECU:LENGTH_SHORT" />
</ECEntityClass>

<ECEntityClass typeName="ProfiledSolidInformationRecordElement" modifier="Abstract" displayLabel="Profiled Solid Information Record Element" description="A bis:InformationRecordElement serving as the base class for implementations capturing data about solids defined from Profiles.">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Markdown was not updated

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Last time we discussed these changes at the BWG, updating documentation was left to your team to address once these new classes and patterns are proved to work for your use-cases.

@diegoalexdiaz diegoalexdiaz requested a review from a team as a code owner August 4, 2022 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants