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

Narrow errors for neo datetime #4875

Merged
merged 2 commits into from
May 7, 2024

Conversation

robertbastian
Copy link
Member

@robertbastian robertbastian commented May 7, 2024

Part of #4336

Per policy in #4638 (comment)

@robertbastian robertbastian requested review from sffc and removed request for zbraniecki and nordzilla May 7, 2024 18:09
) -> Result<FixedDecimalFormatter, DataError> {
FixedDecimalFormatter::try_new(locale, options).map_err(|e| match e {
DecimalError::Data(e) => e,
_ => unreachable!(),
Copy link
Member Author

@robertbastian robertbastian May 7, 2024

Choose a reason for hiding this comment

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

this unreachable branch will disappear in 2.0, when the external constructor can return DataError only. Similar below

@robertbastian robertbastian requested a review from a team as a code owner May 7, 2024 18:19
@sffc
Copy link
Member

sffc commented May 7, 2024

@robertbastian Can you reference the corresponding issue in the OP?

@robertbastian
Copy link
Member Author

Done

@@ -952,7 +994,7 @@ impl RawDateTimeNames {
///// Textual symbols /////
FieldSymbol::Era => {
self.load_year_names(
year_provider.ok_or(Error::MissingNames(field))?,
year_provider.ok_or(LoadError::MissingNames(field))?,
Copy link
Member

Choose a reason for hiding this comment

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

Thought: With the work I'm doing to consolidate the neo formatter types, this error case might go away which would be nice.

@sffc
Copy link
Member

sffc commented May 7, 2024

@robertbastian Also please link the issue about constructor errors, where we listed out the specific cases where returning DataError directly from constructors was acceptable

@robertbastian robertbastian merged commit 51bd08e into unicode-org:main May 7, 2024
30 checks passed
@robertbastian robertbastian deleted the neoerrors branch May 7, 2024 18:39
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.

2 participants