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

Full audit of datetime renames #5837

Open
8 of 9 tasks
sffc opened this issue Nov 19, 2024 · 6 comments
Open
8 of 9 tasks

Full audit of datetime renames #5837

sffc opened this issue Nov 19, 2024 · 6 comments
Labels
C-datetime Component: datetime, calendars, time zones

Comments

@sffc
Copy link
Member

sffc commented Nov 19, 2024

For 2.0 Beta:

  • NeoSkeletonLength should be Length
  • There were some questions about the name of IsAnyCalendarKind
  • There should be a format_iso function on DateTimeFormatter, possibly replacing convert_and_format
  • strict_format should maybe be format_same_calendar
  • FormattedNeoDateTime should be FormattedDateTime
  • DateTimePatternFormatter should have a single generic format function
  • DateTimePatternFormatter and friends should use FSet instead of R
  • DateTimeNames::with_pattern should be try_with_pattern and with_pattern_unchecked (Consider exact policy around DateTimeError in datetime formatting #4336 (comment))

For 2.0 Final:

  • The markers should be renamed. They mostly still have "neo" in the name and/or the module
@sffc sffc added the C-datetime Component: datetime, calendars, time zones label Nov 19, 2024
@sffc sffc added this to the ICU4X 2.0 Beta ⟨P1⟩ milestone Nov 19, 2024
@Manishearth
Copy link
Member

For 2.0 Final I would like every function involving fieldsets to have docs pointing to the fieldset module. Plus every trait like AllAnyCalendarFormattingDataMarkers should explain what its common bounds mean i nrelation to fieldsets, and what to do when the trait isn't implemented.

@Manishearth
Copy link
Member

Manishearth commented Nov 19, 2024

(I'm in favor of all bullet points here. Replacing convert_and_format seems fine, we only have it because that was the initial behavior of the module and we wanted to keep it)

@sffc
Copy link
Member Author

sffc commented Nov 19, 2024

There were some questions about the name of IsAnyCalendarKind

Did you want to weigh in on what this trait should be called? I had recorded a comment that someone didn't like the name but didn't have an alternative. I'm personally still happy with the name.

@Manishearth
Copy link
Member

No strong opinions, fine with the name. IsConcreteCalendar perhaps.

@sffc
Copy link
Member Author

sffc commented Nov 19, 2024

No strong opinions, fine with the name. IsConcreteCalendar perhaps.

I'd kinda like to move it to icu_calendar since it is in the same realm of functionality as IntoAnyCalendar. Can you suggest an appropriate trait signature?

@Manishearth
Copy link
Member

I'd say the same one?

One thing to note is that Calendar already has this functionality, what this trait gets us is rather datetime-specific: "if you care about a calendar, is this the right calendar, or no?" I don't see that as particularly useful for calendar consumers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-datetime Component: datetime, calendars, time zones
Projects
None yet
Development

No branches or pull requests

2 participants