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

Backward compatibility for removed Engine.Create methods #164

Merged
merged 2 commits into from
Nov 15, 2019

Conversation

adecler
Copy link
Member

@adecler adecler commented Nov 13, 2019

Issues addressed by this PR

Closes #163

After an attempt of solving this problem within the Versioning_Toolkit, I ended up taking care of it directly in the BHoM_UI. The main reason for it is the lack of return type stored in the json version of a MethodInfo. I tried to work with method Name and namespace to identify the return type but there was a few too many exceptions for my liking. Solving it here felt more robust and is after all a UI specific versioning issue. This doesn't change the fact that I will add a support for explicitly deprecated method in the versioning engine as planned.

Test files

  • Create some CreateObject components (make sure they use a specific Engine.Create method) and save the file.
  • Comment out the corresponding Create methods in the Engine and recompile
  • Reopened the saved script file and make sure the component is still working and now has the ability to remove inputs and an input selector menu.

Additional comments

For the input selection menu to remove pre-existing inputs properly, make sure to use the corresponding PR in Grasshopper_Toolkit. For Dynamo, the equivalent fix has been pushed to the pending PR.

@adecler adecler added the type:feature New capability or enhancement label Nov 13, 2019
@adecler adecler added this to the BHoM 3.0 β MVP milestone Nov 13, 2019
@adecler adecler self-assigned this Nov 13, 2019
Copy link
Member

@epignatelli epignatelli left a comment

Choose a reason for hiding this comment

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

LGTM!

Tested in Grasshopper with the IdentityFragment in the MEP_Engine.
image

@al-fisher
Copy link
Member

Solving it here felt more robust and is after all a UI specific versioning issue. This doesn't change the fact that I will add a support for explicitly deprecated method in the versioning engine as planned.

Yep. Makes sense @adecler - thanks

Also - I don't know if you have already explicitly added UI deprecation as an additional use case to versioning going forward (in addition to the namespace, type, method, internal property cases etc.)
Will be good to add and summarise - there will always continue to be tweaks to the UI going forward which we will need to support backwards compatibility for.
However - agree there is significant difference between Versioning of oM and Engine, than rest of BHoM. With UI potentially requiring more responsibility for future changes in UI itself and Parent Adapter etc. Good to clarify all those potential cases and the lines between them.

@adecler
Copy link
Member Author

adecler commented Nov 15, 2019

/azp run BHoM_UI.CheckInstaller

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@adecler
Copy link
Member Author

adecler commented Nov 15, 2019

/azp run BHoM_UI.CheckInstaller

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:feature New capability or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Backward compatibility for removed Engine.Create methods
3 participants