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

BHoM_Engine: Additional check for assembly qualified type without version for Create EngineType #3399

Merged

Conversation

IsakNaslundBh
Copy link
Contributor

Issues addressed by this PR

Closes #3398

Add additional check for assembly qualified name without version. Important for edgecase of same namespace and typename, from different assemblies for types serialized in an older major version. Only runs this additional step if more than one type is found.

Generally having this duplication should most likely be avoided, but as that has not been enforced, and was not an issue before #3389 which fixed other critical issues, but introduced this edge case problem, we should still attempt to make sure it works after that fix.

Test files

All query components should deserialise correctly:

On Sharepointhttps://burohappold.sharepoint.com/:u:/r/sites/BHoM/02_Current/12_Scripts/03_Alpha/BuroHappoldEngineering/ModelLaundry_Toolkit/[BuroHappold_BHoM_v3.3.beta_Structures_ML_BasicWorkFlow.gh](https://burohappold.sharepoint.com/:u:/r/sites/BHoM/02_Current/12_Scripts/03_Alpha/BuroHappoldEngineering/ModelLaundry_Toolkit/BuroHappold_BHoM_v3.3.beta_Structures_ML_BasicWorkFlow.gh?csf=1&web=1&e=6E7xl3)?csf=1&web=1&e=6E7xl3

Changelog

Additional comments

@IsakNaslundBh IsakNaslundBh added the type:bug Error or unexpected behaviour label Aug 22, 2024
@IsakNaslundBh IsakNaslundBh self-assigned this Aug 22, 2024
@IsakNaslundBh
Copy link
Contributor Author

@BHoMBot check compliance

Copy link

bhombot-ci bot commented Aug 22, 2024

@IsakNaslundBh to confirm, the following actions are now queued:

  • check code-compliance
  • check documentation-compliance
  • check project-compliance
  • check branch-compliance
  • check dataset-compliance
  • check copyright-compliance

There are 1 requests in the queue ahead of you.

@IsakNaslundBh
Copy link
Contributor Author

@BHoMBot check unit-tests
@BHoMBot check required

Copy link

bhombot-ci bot commented Aug 22, 2024

@IsakNaslundBh to confirm, the following actions are now queued:

  • check unit-tests
  • check code-compliance
  • check documentation-compliance
  • check project-compliance
  • check core
  • check null-handling
  • check serialisation
  • check versioning
  • check installer

There are 1 requests in the queue ahead of you.

Copy link

bhombot-ci bot commented Aug 22, 2024

The check code-compliance has already been run previously and recorded as a successful check. This check has not been run again at this time.

Copy link

bhombot-ci bot commented Aug 22, 2024

The check documentation-compliance has already been run previously and recorded as a successful check. This check has not been run again at this time.

@IsakNaslundBh
Copy link
Contributor Author

@BHoMBot check versioning

Copy link

bhombot-ci bot commented Aug 22, 2024

@IsakNaslundBh to confirm, the following actions are now queued:

  • check versioning

Copy link
Member

@pawelbaran pawelbaran left a comment

Choose a reason for hiding this comment

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

The PR looks good, I tested it both in GH and Revit with positive results. One minor code comment left, besides that all good 👍

BHoM_Engine/Create/Type/EngineType.cs Outdated Show resolved Hide resolved
@IsakNaslundBh
Copy link
Contributor Author

@BHoMBot check compliance
@BHoMBot check required
@BHoMBot check unit-tests

Copy link

bhombot-ci bot commented Aug 28, 2024

@IsakNaslundBh to confirm, the following actions are now queued:

  • check code-compliance
  • check documentation-compliance
  • check project-compliance
  • check branch-compliance
  • check dataset-compliance
  • check copyright-compliance
  • check code-compliance
  • check documentation-compliance
  • check project-compliance
  • check core
  • check null-handling
  • check serialisation
  • check versioning
  • check installer
  • check unit-tests

Copy link

bhombot-ci bot commented Aug 28, 2024

The check code-compliance has already been run previously and recorded as a successful check. This check has not been run again at this time.

Copy link

bhombot-ci bot commented Aug 28, 2024

The check documentation-compliance has already been run previously and recorded as a successful check. This check has not been run again at this time.

Copy link
Member

@pawelbaran pawelbaran left a comment

Choose a reason for hiding this comment

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

Happy to approve following code review and testing 👍

@pawelbaran
Copy link
Member

@BHoMBot check ready-to-merge

Copy link

bhombot-ci bot commented Aug 28, 2024

@pawelbaran to confirm, the following actions are now queued:

  • check ready-to-merge

@pawelbaran pawelbaran merged commit bd8c7d3 into develop Aug 28, 2024
13 checks passed
@pawelbaran pawelbaran deleted the BHoM_Engine-#3398-FixIssuesWithFindingCorrectEnigneType branch August 28, 2024 07:53
@BHoMBot BHoMBot mentioned this pull request Sep 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Error or unexpected behaviour
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BHoM_Engine: Fix issue with finding correct engine type for duplicate namespace across repositories
2 participants