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

break(objects): adds new convert to native displayable bindings #2846

Merged
merged 6 commits into from
Aug 16, 2023

Conversation

clairekuang
Copy link
Member

@clairekuang clairekuang commented Aug 14, 2023

Description & motivation

THIS IS A BREAKING CHANGE

Adds new ConvertToNativeDisplayable() and CanConvertToNativeDisplayable() bindings to our ISpeckleConverter interface.
These bindings are a prerequisite to pr #2841 and part of #2777

Changes:

Converters

Checklist:

  • ⚠️This breaking change should be communicated to our dev users and community contributed connectors.

@clairekuang clairekuang requested a review from AlanRynne August 14, 2023 10:08
@clairekuang clairekuang added the objects issues related to the objects kit. label Aug 14, 2023
Copy link
Contributor

@AlanRynne AlanRynne left a comment

Choose a reason for hiding this comment

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

Everything looks right on my end, pending the resolution of the Archicad CI problem (which seems unrelated to this)

@teocomi teocomi self-requested a review August 14, 2023 15:30
@AlanRynne AlanRynne changed the title feat(objects): adds new convert to native displayable bindings break(objects): adds new convert to native displayable bindings Aug 14, 2023
@AlanRynne AlanRynne added core issues related to the .net sdk. enhancement New feature or request labels Aug 15, 2023
Comment on lines 58 to 76
/// <summary>
/// Converts a Speckle object to a native displayable one
/// </summary>
/// <param name="object">Speckle object to convert</param>
/// <returns></returns>
public object ConvertToNativeDisplayable(Base @object);

/// <summary>
/// Checks if it can convert a Speckle object to a native one
/// </summary>
/// <param name="object">Speckle object to convert</param>
/// <returns></returns>
public bool CanConvertToNative(Base @object);

/// <summary>
/// Checks if it can convert a Speckle object to a native displayable one
/// </summary>
/// <param name="object">Speckle object to convert</param>
/// <returns></returns>
Copy link
Contributor

@AlanRynne AlanRynne Aug 15, 2023

Choose a reason for hiding this comment

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

Regarding the xml docs: Maybe we should be more thorough on the explanation of what this is intended for, to prevent misusage in the near future by us or others.
Current one seems rather vague. It also seems it's not correctly defined for me:
What this does is convert a "displayable Speckle Object" into native; rather than convert a "generic speckle object" into "native displayable".

@AlanRynne
Copy link
Contributor

Updated branch from dev to fix archicad failing CI

@clairekuang clairekuang merged commit 20870df into dev Aug 16, 2023
@clairekuang clairekuang deleted the objects/claire/displayable-convertr-bindings branch August 16, 2023 09:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core issues related to the .net sdk. enhancement New feature or request objects issues related to the objects kit.
Projects
No open projects
Status: 🧪 Merged (Test)
Development

Successfully merging this pull request may close these issues.

3 participants