-
Notifications
You must be signed in to change notification settings - Fork 174
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
DUI3-71 receive bindings and receive geometry #3544
Conversation
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.
Only comments on binding for now, will test converters later and review again.
DUI3-DX/Connectors/Revit/Speckle.Connectors.RevitShared/Bindings/RevitReceiveBinding.cs
Outdated
Show resolved
Hide resolved
DUI3-DX/Connectors/Revit/Speckle.Connectors.RevitShared/Bindings/RevitReceiveBinding.cs
Outdated
Show resolved
Hide resolved
.../Connectors/Revit/Speckle.Connectors.RevitShared/DependencyInjection/RevitConnectorModule.cs
Outdated
Show resolved
Hide resolved
...Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Receive/RevitHostObjectBuilder.cs
Outdated
Show resolved
Hide resolved
DUI3-DX/Converters/Revit/Speckle.Converters.RevitShared/RevitRootToSpeckleConverter.cs
Show resolved
Hide resolved
...Converters/Revit/Speckle.Converters.RevitShared/ToHost/Raw/Geometry/ICurveConverterToHost.cs
Show resolved
Hide resolved
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.
Seems pretty good. I have some concern around the correct lifetime for the transaction (but i could be fine). I've also some points where I'd like @clairekuang to be involved and perhaps for between you, you raise any necessary tickets for gaps.
DUI3-DX/Connectors/Revit/Speckle.Connectors.RevitShared/Bindings/RevitReceiveBinding.cs
Show resolved
Hide resolved
...Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Receive/RevitHostObjectBuilder.cs
Outdated
Show resolved
Hide resolved
DUI3-DX/Connectors/Revit/Speckle.Connectors.RevitShared/Bindings/RevitReceiveBinding.cs
Show resolved
Hide resolved
DUI3-DX/Connectors/Revit/Speckle.Connectors.RevitShared/Bindings/RevitReceiveBinding.cs
Show resolved
Hide resolved
...Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Receive/RevitHostObjectBuilder.cs
Show resolved
Hide resolved
...nverters/Revit/Speckle.Converters.RevitShared/ToHost/Raw/Geometry/PolylineConverterToHost.cs
Outdated
Show resolved
Hide resolved
...nverters/Revit/Speckle.Converters.RevitShared/ToHost/Raw/Geometry/PolylineConverterToHost.cs
Outdated
Show resolved
Hide resolved
...Revit/Speckle.Converters.RevitShared/ToHost/TopLevel/ModelCurveToSpeckleTopLevelConverter.cs
Outdated
Show resolved
Hide resolved
...Revit/Speckle.Converters.RevitShared/ToHost/TopLevel/ModelCurveToSpeckleTopLevelConverter.cs
Show resolved
Hide resolved
...Revit/Speckle.Converters.RevitShared/ToHost/TopLevel/ModelCurveToSpeckleTopLevelConverter.cs
Show resolved
Hide resolved
Oh yeah and the Dispose()/lack of using/disabling the warning |
...nverters/Revit/Speckle.Converters.RevitShared/ToHost/Raw/Geometry/PolylineConverterToHost.cs
Outdated
Show resolved
Hide resolved
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.
There's still a couple of points
...-DX/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Receive/TransactionManager.cs
Show resolved
Hide resolved
DUI3-DX/Converters/Revit/Speckle.Converters.RevitShared/Services/ScalingServiceToHost.cs
Show resolved
Hide resolved
...nverters/Revit/Speckle.Converters.RevitShared/ToHost/Raw/Geometry/PolylineConverterToHost.cs
Show resolved
Hide resolved
...nverters/Revit/Speckle.Converters.RevitShared/ToHost/Raw/Geometry/PolylineConverterToHost.cs
Show resolved
Hide resolved
...Revit/Speckle.Converters.RevitShared/ToHost/TopLevel/ModelCurveToSpeckleTopLevelConverter.cs
Show resolved
Hide resolved
changes were addressed
Description & motivation
Changes:
Connectors / Revit
Converter / Revit
Converter / Revit / ToHost
Converter / Revit / ToSpeckle
To-do before merge:
Screenshots:
Validation of changes:
receiving works for model lines of various types
Checklist:
References