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

Geometry_Engine: Removing Radius(Arc) and adding warnings to Area() methods #2262

Merged
merged 3 commits into from
Jan 13, 2021

Conversation

LMarkowski
Copy link
Contributor

@LMarkowski LMarkowski commented Jan 11, 2021

Issues addressed by this PR

Closes #945
Closes #946

Deprecating Query.Radius(Arc) as Radius is a public property of the Arc class.
EDIT
Decided to merge this one with #946 as they both update Area.cs and I didn't want to wait until this one get merged 😉
/EDIT

Test files

945
Not needed as it doesn't affect a logic of any method, however here is the script I used to check if any reference has been missed.
946
Not needed - only added warnings.

Changelog

  • Deprecating Query.Radius(Arc) method in the Geometry_Engine.
  • Updating methods in Geometry_Engine to call for Radius property instead of Radius() method.
  • Adding warnings to Query.Area methods in the Geometry_Engine in cases of open curves.

@LMarkowski LMarkowski added the type:compliance Non-conforming to code guidelines label Jan 11, 2021
@LMarkowski LMarkowski requested a review from pawelbaran January 11, 2021 13:45
@LMarkowski LMarkowski self-assigned this Jan 11, 2021
@FraserGreenroyd
Copy link
Contributor

@BHoMBot check compliance

@bhombot-ci
Copy link

bhombot-ci bot commented Jan 11, 2021

@FraserGreenroyd to confirm, check-code-compliance, check-documentation-compliance, check-project-compliance, check-branch-compliance, check-dataset-compliance, and, if applicable, check-copyright-compliance tasks are now queued.

@FraserGreenroyd
Copy link
Contributor

@BHoMBot check documentation compliance

@bhombot-ci
Copy link

bhombot-ci bot commented Jan 11, 2021

@FraserGreenroyd to confirm, check-documentation-compliance task is now queued.

@LMarkowski
Copy link
Contributor Author

@BHoMBot check core

@bhombot-ci
Copy link

bhombot-ci bot commented Jan 12, 2021

@LMarkowski to confirm, check-core task is now queued.

@LMarkowski
Copy link
Contributor Author

/azp run BHoM_Engine.CheckCore

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@LMarkowski LMarkowski added the status:WIP PR in progress and still in draft, not ready for formal review label Jan 12, 2021
@LMarkowski LMarkowski changed the title Geometry_Engine: Removing Radius(Arc) Geometry_Engine: Removing Radius(Arc) and adding warnings to Area() methods Jan 12, 2021
@LMarkowski LMarkowski removed the status:WIP PR in progress and still in draft, not ready for formal review label Jan 12, 2021
@LMarkowski
Copy link
Contributor Author

@BHoMBot check compliance

@bhombot-ci
Copy link

bhombot-ci bot commented Jan 12, 2021

@LMarkowski to confirm, check-code-compliance, check-documentation-compliance, check-project-compliance, check-branch-compliance, check-dataset-compliance, and, if applicable, check-copyright-compliance tasks are now queued.

@LMarkowski
Copy link
Contributor Author

/azp run BHoM_Engine.CheckInstaller

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@IsakNaslundBh
Copy link
Contributor

Should get into the Habit of putting testiles (even small as the one you used @LMarkowski ) on sharepoint, in a folder easily identifiable with the PR. I put the file you shared as well as a quick test that the area methods are still working and messages fire correctly here @LMarkowski :

https://burohappold.sharepoint.com/:f:/s/BHoM/EvJEY1vfAu1Mpeg-CRFlRggBYeuOoNnKvhOyBXim6UYydw?e=OTFHc6

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.

Code change looks good to me.

@IsakNaslundBh IsakNaslundBh merged commit 0dc2bf0 into master Jan 13, 2021
@IsakNaslundBh IsakNaslundBh deleted the #945-RemoveRadius(Arc) branch January 13, 2021 12:06
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
3 participants