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

Promote IInitialisationSettings interface to BHoM #480

Merged
merged 2 commits into from
Feb 15, 2024

Conversation

FraserGreenroyd
Copy link
Contributor

@FraserGreenroyd FraserGreenroyd commented Feb 14, 2024

NOTE: Depends on

BHoM/BHoM#1598
#480

Issues addressed by this PR

Fixes BHoM/BHoMAnalytics_Toolkit#73

BHoM_UI has not been upgraded to NetStandard2.0 because many of the GUI components are not compatible. The BHoM_UI project is typically only depended upon by the other UIs (Grasshopper, Excel, etc.) which are also not NetStandard2.0 compliant. However, BHoMAnalytics_Toolkit, and others, depend on the UI_oM and UI_Engine, and particularly as @adecler continues prototyping up our zero code tools, they require NetStandard2.0 DLLs, and the issue as described in BHoM/BHoMAnalytics_Toolkit#73 is basically the Analytics Toolkit is unhappy because the UI elements it depends on are in NetFramework rather than NetStandard.

Thus, UI_oM and UI_Engine have been upgraded to NetStandard2.0 while BHoM_UI has been left on NetFramework but given some tweaks to support it.

I have then added the alternative configuration for ZeroCodeTool which will be used when building the Nugets to exclude the BHoM_UI project when it builds for Nugets.

To test this, compile this PR and then compile a UI of your choice (Grasshopper, Excel, etc.) to ensure it is built on top of this PR, and then play around with the UI. Try and use the search menus and stuff which rely on the NetFramework elements more to ensure they aren't broken.

While all of the above is still true and valid, testing revealed too many issues owing to the need for the UIs to remain within the NetFramework era of the runtime frameworks. Therefore, upgrading the oM and Engine did not work as intended and caused issues when using the UIs.

So instead I went back to the original toolkit - BHoMAnalytics_Toolkit - and questioned why it was depending on BHoM_UI in the first place. It turns out, it's entirely for the interface IInitialisationSettings so that BHoMAnalytics could have their own settings object to be handled on initialisation. As great as that is, a dependency on UI_oM for just that seems overkill, especially as BHoMAnaltyics_Toolkit is not a UI itself, nor does it have a UI, and thus could arguably not depend on BHoM_UI for its work.

Thus, this solution - move the interface it needs from here up to the core BHoM repository within the base oM (BHoM). This makes sense from my perspective as while initialisation settings are nice to use from a UI, they are not only needed from a UI context, so having them in the core makes more sense to be more accessible for contexts outside the UI.

@FraserGreenroyd FraserGreenroyd added the type:compliance Non-conforming to code guidelines label Feb 14, 2024
@FraserGreenroyd FraserGreenroyd self-assigned this Feb 14, 2024
@FraserGreenroyd FraserGreenroyd changed the title B ho m UI #73 upgrade to net standard Upgrade UI_oM and UI_Engine to NetStandard2.0 Feb 14, 2024
@FraserGreenroyd
Copy link
Contributor Author

@BHoMBot check project-compliance

Copy link

bhombot-ci bot commented Feb 14, 2024

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

  • check project-compliance

Copy link

bhombot-ci bot commented Feb 14, 2024

@FraserGreenroyd fix requested for project compliance.

The errors with the CSProject (.csproj) files have been recorded as annotations on the checks tab.

I will apply the fixes to every case detailed on the checks tab with the exception of any references to the target framework. I am unable to provide fixes to the Target Framework automatically, these will need to be performed manually. If you want to perform the fixes in a different manner please resolve this manually and rerun the check.

If you are happy for me to go ahead and perform this action, please reply with:

@BHoMBot fix project file ref. 21569055403

@FraserGreenroyd
Copy link
Contributor Author

@BHoMBot fix project file ref. 21569055403

Copy link

bhombot-ci bot commented Feb 14, 2024

@FraserGreenroyd I have queued up your request to fix the csproj file(s). There are 0 requests in the queue ahead of you.

Copy link

bhombot-ci bot commented Feb 14, 2024

@FraserGreenroyd I am now going to fix the project compliance in accordance with the annotations previously made.

Copy link

bhombot-ci bot commented Feb 14, 2024

@FraserGreenroyd to confirm I have now resolved the project compliance issues and pushed a commit to this Pull Request.

@FraserGreenroyd
Copy link
Contributor Author

@BHoMBot check project-compliance

Copy link

bhombot-ci bot commented Feb 14, 2024

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

  • check project-compliance

@FraserGreenroyd
Copy link
Contributor Author

@BHoMBot check versioning
@BHoMBot check installer
@BHoMBot check core
@BHoMBot check copyright-compliance
@BHoMBot check project-compliance

Copy link

bhombot-ci bot commented Feb 14, 2024

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

  • check versioning
  • check installer
  • check core
  • check copyright-compliance
  • check project-compliance

@FraserGreenroyd FraserGreenroyd force-pushed the BHoM_UI-#73-UpgradeToNetStandard branch from e2bf974 to c86ac03 Compare February 14, 2024 17:38
@FraserGreenroyd FraserGreenroyd changed the title Upgrade UI_oM and UI_Engine to NetStandard2.0 Promote IInitialisationSettings interface to BHoM Feb 14, 2024
Copy link

bhombot-ci bot commented Feb 14, 2024

@FraserGreenroyd just to let you know, I have provided a check-versioning result to this Pull Request as it was detected to be linked to other Pull Requests in a series. The comment which triggered this check came from @FraserGreenroyd on BHoM

Copy link

bhombot-ci bot commented Feb 14, 2024

@FraserGreenroyd just to let you know, I have provided a check-installer result to this Pull Request as it was detected to be linked to other Pull Requests in a series. The comment which triggered this check came from @FraserGreenroyd on BHoM

@FraserGreenroyd
Copy link
Contributor Author

@BHoMBot check compliance
@BHoMBot check core

Copy link

bhombot-ci bot commented Feb 14, 2024

@FraserGreenroyd 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 core

There are 8 requests in the queue ahead of you.

Copy link

bhombot-ci bot commented Feb 14, 2024

@FraserGreenroyd just to let you know, I have provided a check-installer result to this Pull Request as it was detected to be linked to other Pull Requests in a series. The comment which triggered this check came from @FraserGreenroyd on BHoMAnalytics_Toolkit

@FraserGreenroyd
Copy link
Contributor Author

@BHoMBot check copyright-compliance
@BHoMBot check project-compliance

Copy link

bhombot-ci bot commented Feb 14, 2024

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

  • check copyright-compliance
  • check project-compliance

There are 5 requests in the queue ahead of you.

Copy link

bhombot-ci bot commented Feb 14, 2024

@FraserGreenroyd just to let you know, I have provided a check-versioning result to this Pull Request as it was detected to be linked to other Pull Requests in a series. The comment which triggered this check came from @FraserGreenroyd on BHoMAnalytics_Toolkit

@FraserGreenroyd
Copy link
Contributor Author

@BHoMBot check core

Copy link

bhombot-ci bot commented Feb 14, 2024

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

  • check core

There are 5 requests in the queue ahead of you.

Copy link

bhombot-ci bot commented Feb 14, 2024

@FraserGreenroyd just to let you know, I have provided a check-installer result to this Pull Request as it was detected to be linked to other Pull Requests in a series. The comment which triggered this check came from @FraserGreenroyd on BHoMAnalytics_Toolkit

Copy link
Contributor

@Tom-Kingstone Tom-Kingstone left a comment

Choose a reason for hiding this comment

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

Approval based off my review on linked PR.

@Tom-Kingstone
Copy link
Contributor

@BHoMBot check ready-to-merge

Copy link

bhombot-ci bot commented Feb 15, 2024

@Tom-Kingstone to confirm, the following actions are now queued:

  • check ready-to-merge

Copy link

bhombot-ci bot commented Feb 15, 2024

@FraserGreenroyd just to let you know, I have provided a check-ready-to-merge result to this Pull Request as it was detected to be linked to other Pull Requests in a series. The comment which triggered this check came from @Tom-Kingstone on BHoM

@FraserGreenroyd FraserGreenroyd merged commit bb94b45 into develop Feb 15, 2024
6 checks passed
@FraserGreenroyd FraserGreenroyd deleted the BHoM_UI-#73-UpgradeToNetStandard branch February 15, 2024 09:26
@bhombot-ci bhombot-ci bot mentioned this pull request Mar 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:compliance Non-conforming to code guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing dependency to BHoM_UI in the NuGet package for net standard 2.0
2 participants