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: replaced calls to Query.Clone() and Query.IClone() with calls to BH.Engine.Base.DeepClone() #2585

Merged
merged 6 commits into from
Jul 30, 2021

Conversation

alelom
Copy link
Member

@alelom alelom commented Jul 30, 2021

Issues addressed by this PR

Closes #2584
Closes #2583
Closes #2582
Closes #2577

Closes #2576

Test files

Changelog

Additional comments

@alelom
Copy link
Member Author

alelom commented Jul 30, 2021

@BHoMBot check installer

@bhombot-ci
Copy link

bhombot-ci bot commented Jul 30, 2021

@alelom to confirm, the following checks are now queued:

  • installer

@alelom
Copy link
Member Author

alelom commented Jul 30, 2021

@BHoMBot check installer

@bhombot-ci
Copy link

bhombot-ci bot commented Jul 30, 2021

@alelom to confirm, the following checks are now queued:

  • installer

There are 29 requests in the queue ahead of you.

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.

For the versioning @alelom I think we should put this on the DeepClone method:

[PreviousVersion("4.3", "BH.Engine.Geometry.Query.Clone(BH.oM.Geometry.Plane)")]
[PreviousVersion("4.3", "BH.Engine.Geometry.Query.Clone(BH.oM.Geometry.Point)")]
[PreviousVersion("4.3", "BH.Engine.Geometry.Query.Clone(BH.oM.Geometry.Vector)")]
[PreviousVersion("4.3", "BH.Engine.Geometry.Query.Clone(BH.oM.Geometry.CoordinateSystem.Cartesian)")]
[PreviousVersion("4.3", "BH.Engine.Geometry.Query.Clone(BH.oM.Geometry.Arc)")]
[PreviousVersion("4.3", "BH.Engine.Geometry.Query.Clone(BH.oM.Geometry.Circle)")]
[PreviousVersion("4.3", "BH.Engine.Geometry.Query.Clone(BH.oM.Geometry.Ellipse)")]
[PreviousVersion("4.3", "BH.Engine.Geometry.Query.Clone(BH.oM.Geometry.Line)")]
[PreviousVersion("4.3", "BH.Engine.Geometry.Query.Clone(BH.oM.Geometry.NurbsCurve)")]
[PreviousVersion("4.3", "BH.Engine.Geometry.Query.Clone(BH.oM.Geometry.PolyCurve)")]
[PreviousVersion("4.3", "BH.Engine.Geometry.Query.Clone(BH.oM.Geometry.Polyline)")]
[PreviousVersion("4.3", "BH.Engine.Geometry.Query.Clone(BH.oM.Geometry.Extrusion)")]
[PreviousVersion("4.3", "BH.Engine.Geometry.Query.Clone(BH.oM.Geometry.Loft)")]
[PreviousVersion("4.3", "BH.Engine.Geometry.Query.Clone(BH.oM.Geometry.NurbsSurface)")]
[PreviousVersion("4.3", "BH.Engine.Geometry.Query.Clone(BH.oM.Geometry.PlanarSurface)")]
[PreviousVersion("4.3", "BH.Engine.Geometry.Query.Clone(BH.oM.Geometry.Pipe)")]
[PreviousVersion("4.3", "BH.Engine.Geometry.Query.Clone(BH.oM.Geometry.PolySurface)")]
[PreviousVersion("4.3", "BH.Engine.Geometry.Query.Clone(BH.oM.Geometry.Mesh)")]
[PreviousVersion("4.3", "BH.Engine.Geometry.Query.Clone(BH.oM.Geometry.Face)")]
[PreviousVersion("4.3", "BH.Engine.Geometry.Query.Clone(BH.oM.Geometry.BoundingBox)")]
[PreviousVersion("4.3", "BH.Engine.Geometry.Query.Clone(BH.oM.Geometry.CompositeGeometry)")]
[PreviousVersion("4.3", "BH.Engine.Geometry.Query.Clone(BH.oM.Geometry.Quaternion)")]
[PreviousVersion("4.3", "BH.Engine.Geometry.Query.Clone(BH.oM.Geometry.TransformMatrix)")]
[PreviousVersion("4.3", "BH.Engine.Geometry.Query.Clone(BH.oM.Geometry.Basis)")]
[PreviousVersion("4.3", "BH.Engine.Geometry.Query.Clone(BH.oM.Geometry.SurfaceTrim)")]
[PreviousVersion("4.3", "BH.Engine.Geometry.Query.IClone(BH.oM.Geometry.IGeometry)")]
[PreviousVersion("4.3", "BH.Engine.Geometry.Query.IClone(BH.oM.Geometry.ICurve)")]
[PreviousVersion("4.3", "BH.Engine.Geometry.Query.IClone(BH.oM.Geometry.ISurface)")]

Just so scripts and stuff can version nicely to using DeepClone as well, what do you think?

@alelom
Copy link
Member Author

alelom commented Jul 30, 2021

@BHoMBot check required

@bhombot-ci
Copy link

bhombot-ci bot commented Jul 30, 2021

@alelom to confirm, the following checks are now queued:

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

There are 11 requests in the queue ahead of you.

@FraserGreenroyd
Copy link
Contributor

@BHoMBot check versioning

@bhombot-ci
Copy link

bhombot-ci bot commented Jul 30, 2021

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

  • versioning

@FraserGreenroyd
Copy link
Contributor

@BHoMBot check versioning

@bhombot-ci
Copy link

bhombot-ci bot commented Jul 30, 2021

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

  • versioning

@FraserGreenroyd
Copy link
Contributor

@BHoMBot check copyright-compliance
@BHoMBot check dataset-compliance

@bhombot-ci
Copy link

bhombot-ci bot commented Jul 30, 2021

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

  • copyright-compliance
  • dataset-compliance

@bhombot-ci
Copy link

bhombot-ci bot commented Jul 30, 2021

FAO: @FraserGreenroyd
@FraserGreenroyd is seeking dispensation on this Pull Request to skip a required check. Please can you provide authorisation for the check to be skipped, or provide assistance as appropriate.

The check they wish to have dispensation on is documentation-compliance.

If you are providing dispensation on this occasion, please reply with:

@BHoMBot this is a CI/CD instruction. I am authorising dispensation to be granted on check ref. 3201609309

@FraserGreenroyd
Copy link
Contributor

@BHoMBot this is a CI/CD instruction. I am authorising dispensation to be granted on check ref. 3201609309

@bhombot-ci
Copy link

bhombot-ci bot commented Jul 30, 2021

@FraserGreenroyd I have now provided a passing check on reference 3201609309 as requested.

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 have tested this by doing original clones of the geometry and then deep clones and comparing the output via the diffing engine and everything came back equal, suggesting to me that the use of DeepClone is equal to the result of the Geometry IClone and the replacement is safe.

Merging once all checks on dependent PRs are completed.

@FraserGreenroyd
Copy link
Contributor

@BHoMBot check ready-to-merge

@bhombot-ci
Copy link

bhombot-ci bot commented Jul 30, 2021

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

  • ready-to-merge

There are 4 requests in the queue ahead of you.

@FraserGreenroyd FraserGreenroyd merged commit a7ddf98 into master Jul 30, 2021
@FraserGreenroyd FraserGreenroyd deleted the BHoM_Engine-#2576-RemoveGeometryClone branch July 30, 2021 14:20
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
2 participants