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

Make FormattedDateTime Copy and Clone #4476

Merged
merged 3 commits into from
Jan 5, 2024
Merged

Conversation

sffc
Copy link
Member

@sffc sffc commented Dec 20, 2023

Do we want this? We don't have it currently.

Making these Copy/Clone is slightly limiting because then we can't pre-process patterns in the format function.

On the other hand, it is consistent with the ICU4X design principle of intermediate types being ephemeral and throw-away.

Please review and approve @zbraniecki @Manishearth @robertbastian

@sffc sffc requested review from Manishearth and robertbastian and removed request for gregtatum and nordzilla December 20, 2023 00:32
@sffc sffc added the discuss-priority Discuss at the next ICU4X meeting label Dec 20, 2023
Manishearth
Manishearth previously approved these changes Dec 20, 2023
@robertbastian
Copy link
Member

changelog?

@zbraniecki
Copy link
Member

I don't think this is harmful. In 2.0 I would like to explore keeping patterns immutable and iterating over them to compose result on-fly (rather than generating custom patterns in the constructor). This seems compatible with that approach.

@sffc
Copy link
Member Author

sffc commented Jan 5, 2024

OK, no pushback, so I guess I'll merge this ;)

@sffc sffc merged commit f36d948 into unicode-org:main Jan 5, 2024
29 checks passed
@sffc sffc deleted the copyclone branch January 5, 2024 04:49
@sffc sffc removed the discuss-priority Discuss at the next ICU4X meeting label Feb 15, 2024
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.

4 participants