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

Common_Engine: Remove Deprecated Methods #1784

Merged

Conversation

kThorsager
Copy link
Contributor

Issues addressed by this PR

recreating this from #1759 as update from master implied difficulties to merge there.

Closes #1624
Removes all methods in Common_Engine which were moved to Spatial_Engine and adds Versioning for them.

Test files

Common_Engines regular testfiles are here and are upgraded with this PR. i.e. the old labels are removed and they still work.
https://burohappold.sharepoint.com/:f:/s/BHoM/EvyCGXcAkK5OqmqvtfZkZMABtuDNXxKAG7YZdY-dI0SWXA?e=rXXt8h

Changelog

Additional comments

@kThorsager kThorsager added the status:WIP PR in progress and still in draft, not ready for formal review label May 20, 2020
@kThorsager kThorsager added this to the BHoM 3.2 β MVP milestone May 20, 2020
@kThorsager kThorsager self-assigned this May 20, 2020
@kThorsager kThorsager added type:compliance Non-conforming to code guidelines and removed status:WIP PR in progress and still in draft, not ready for formal review labels May 20, 2020
@kThorsager kThorsager marked this pull request as ready for review May 20, 2020 08:48
@kThorsager kThorsager changed the title Common engine #1624 remove deprecated methods v2 Common_Engine: Remove Deprecated Methods May 20, 2020
@IsakNaslundBh
Copy link
Contributor

/azp run BHoM_Engine.CheckCompliance

@IsakNaslundBh
Copy link
Contributor

/azp run BHoM_Engine.CheckInstaller

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

1 similar comment
@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Contributor

@IsakNaslundBh IsakNaslundBh left a comment

Choose a reason for hiding this comment

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

Had to upgrade from master locally for versioning to work (as the upgrade system change had been merged after this PR was created), but after doing so, all test files in Common_Engine upgraded like a charm and worked as expected. Happy to approve, but will hold off merging for a day or two to give someone else a chance to shout before I do. @alelom @FraserGreenroyd @al-fisher

@IsakNaslundBh
Copy link
Contributor

compliance complains about wrong build folder for Common_Engine. Think we can live with that as it should not be used any longer and is about to get removed.

@FraserGreenroyd
Copy link
Contributor

compliance complains about wrong build folder for Common_Engine. Think we can live with that as it should not be used any longer and is about to get removed.

Agreed

Copy link
Contributor

@FraserGreenroyd FraserGreenroyd left a comment

Choose a reason for hiding this comment

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

I was happy with what I saw so am happy to approve. Also happy to wait for more reviews as per @IsakNaslundBh comment before merging.

@IsakNaslundBh IsakNaslundBh merged commit 1915c22 into master May 29, 2020
@IsakNaslundBh IsakNaslundBh deleted the Common_Engine-#1624-Remove-deprecated-methods-v2 branch May 29, 2020 08:46
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.

Common_Engine: Remove deprecated methods
3 participants