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.

Sign up for GitHub

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

Jump to bottom

get rid of Copy() #1730

Open
rade opened this issue Jul 29, 2016 · 2 comments
Open

get rid of Copy() #1730

rade opened this issue Jul 29, 2016 · 2 comments
Labels
tech-debt Unpleasantness that does (or may in future) affect development

Comments

@rade
Copy link
Member

rade commented Jul 29, 2016

Post #1728, we can probably get rid of Copy(), since I suspect it was only used for deep-copying on Merge(), which we no longer do.

Certainly removing Report.Copy() works just fine except for it being used in some tests. Hopefully the same is true of most/all other Copy() functions.
@rade rade added the tech-debt Unpleasantness that does (or may in future) affect development label Jul 29, 2016
@rade rade mentioned this issue Jul 29, 2016
Closed
@rade
Copy link
Member Author

rade commented Jul 29, 2016

Copy() is being used in Topology.With.... functions. Would a shallow copy suffice there?

@rade
Copy link
Member Author

rade commented Jul 29, 2016

Incidentally, a neat trick to implement shallow-copy is to Thing{}.Merge(this).

This was referenced Aug 1, 2016
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
tech-debt Unpleasantness that does (or may in future) affect development
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@rade