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

Convert revit converters to use proxies for unit testing DUI3-149 #3501

Merged
merged 28 commits into from
Jun 14, 2024

Conversation

adamhathcock
Copy link
Member

https://spockle.atlassian.net/browse/DUI3-149

Revit types and proxies exist in new repo: https://github.com/specklesystems/speckle-sharp-host-apis
types and proxies are consumed via Nuget (preview at the moment)

DI registration occurs with new facility to scan repo and add to Autofac as transient objects

Converter assembly only references interfaces
New test assembly made which only references interfaces
Connector assembly references APIs and Interfaces to glue together with DI

Future work:
some clean up in the connector assembly
image

Need to be careful to ensure no breaking. Probably around some casting?

@adamhathcock adamhathcock changed the base branch from main to dui3/alpha June 12, 2024 08:44
@adamhathcock adamhathcock mentioned this pull request Jun 12, 2024
Copy link
Member

@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.

Looks good! just a couple comments but I'm approving meanwhile

Comment on lines +21 to +26
// POC: we probably should not get here without a valid document
// so should this perpetuate or do we assume this is valid?
// relting on the context.UIApplication?.ActiveUIDocument is not right
// this should be some IActiveDocument I suspect?
new DocumentProxy(
context.UIApplication?.ActiveUIDocument?.Document
Copy link
Member

Choose a reason for hiding this comment

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

I think this is an initialisation thing. We may need to delay initialisation of the stack (and maybe the plugin in general) until we know the application has initialised and is no longer null.

If the UIApplication does not exist yet, we can't really do anything else from that point onwards. But I may not have the full picture about this.

Copy link
Member

Choose a reason for hiding this comment

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

I think this is why SharedProjects are giving you so much grief? You should just let the IDE do it's thing... but I may not be right on this one. We should check...

Comment on lines +101 to +104
// POC: why do we need this send selection?
// why does conversion need to know about selection in this way?
public class SendSelection
{
Copy link
Member

Choose a reason for hiding this comment

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

oh WTH 👀

Copy link
Member

Choose a reason for hiding this comment

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

Same! Is this what is breaking your IDE experience?

@adamhathcock adamhathcock merged commit 76c6aba into dui3/alpha Jun 14, 2024
33 checks passed
@adamhathcock adamhathcock deleted the convert-revit-converters branch June 14, 2024 08:26
@adamhathcock adamhathcock changed the title Convert revit converters to use proxies for unit testing Convert revit converters to use proxies for unit testing DUI3-149 Jun 14, 2024
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.

2 participants