Skip to content

Remove the Temporal prefix from Unit, RoundingMode, and UnsignedRoundingMode#254

Merged
nekevss merged 3 commits intomainfrom
remove-temporal-prefix
Apr 15, 2025
Merged

Remove the Temporal prefix from Unit, RoundingMode, and UnsignedRoundingMode#254
nekevss merged 3 commits intomainfrom
remove-temporal-prefix

Conversation

@nekevss
Copy link
Copy Markdown
Member

@nekevss nekevss commented Mar 23, 2025

This PR closes #222.

The background was mentioned in the initial issue. But to restate for this PR, TemporalUnit, TemporalRoundingMode, and TemporalUnsignedRoundingMode come from the very early days of temporal_rs, dating alway back to the initial merge into Boa. The explicit name was to prevent any clashing with Intl. With temporal_rs pulled out of Boa and standing more formally as it's own crate, it's long been overdue to remove the prefix as it is redundant.

The general proposal is to change the below names:

  • temporal_rs::options::TemporalUnit -> temporal_rs::options::Unit
  • temporal_rs::options::TemporalRoundingMode -> temporal_rs::options::RoundingMode
  • temporal_rs::options::TemporalUnsignedRoundingMode -> temporal_rs::options::UnsignedRoundingMode

CC: @Manishearth

The change in this PR also currently extends to temporal_capi. Would there be a preference around keeping the more explicit temporal prefix, or would there not be much of an effect either way in your opinion?

@nekevss nekevss requested a review from jedel1043 March 23, 2025 20:22
@Manishearth
Copy link
Copy Markdown
Contributor

The capi already uses a temporal_rs cpp namespace, so I don't think we need more prefixing.

But we can experiment a bunch with the naming, Diplomat provides rich control over renaming things.

Copy link
Copy Markdown
Member

@jedel1043 jedel1043 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the cleanup!

@nekevss nekevss force-pushed the remove-temporal-prefix branch from 18702b9 to e88adf5 Compare April 15, 2025 03:51
@nekevss nekevss merged commit ca7a601 into main Apr 15, 2025
8 checks passed
@nekevss nekevss deleted the remove-temporal-prefix branch April 15, 2025 04:09
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.

Remove Temporal from option names

3 participants