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

Deep copy time.Time #72

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

josephbuchma
Copy link

Partially resolves #47

@awalterschulze
Copy link
Owner

awalterschulze commented Nov 18, 2021

This doesn't seem to follow the proposed design discussed in #47
I think time is just one case with private fields.
It would great if we could extend this to handle more cases with the design discussed in #47

@josephbuchma
Copy link
Author

I agree. I just quickly added it for myself, and thought why not open PR. Is anyone actively working on generic solution?

@awalterschulze
Copy link
Owner

Currently no one is working on the generic solution, so you are free to go for it, if you would like.

Also if you would like to propose a different solution, we can also discuss in it #47
But, my main concerns with this solution is adding more and more special cases over time, which I would prefer not to support. I hope that makes sense.

@josephbuchma
Copy link
Author

It makes sense. But even with generic solution available, I think it's worth having support for stdlib types like time.Time out of box

@awalterschulze
Copy link
Owner

It makes sense. But even with generic solution available, I think it's worth having support for stdlib types like time.Time out of box

Good point, how many stdlib types are there with private fields? I am sure time.Time is the most popular, but I wonder how many special cases we would have to handle to provide this smooth experience for users.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for time.Time and any type with private fields for DeepCopy?
2 participants