Skip to content

uefi: Make TimeError more descriptive #1211

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

Merged
merged 1 commit into from
Jun 20, 2024

Conversation

andre-braga
Copy link
Contributor

When returning a TimeError, it'd be helpful to specify to the user which field is invalid so that it can be handled accordingly, or at least communicated. This change does this by reimplementing TimeError as an enum instead of a struct, allowing for specification and more verbose error display.

Checklist

  • Sensible git history (for example, squash "typo" or "fix" commits). See the Rewriting History guide for help.
  • Update the changelog (if necessary)

@andre-braga andre-braga reopened this Jun 17, 2024
@andre-braga
Copy link
Contributor Author

Is there a reason for no top level enums in uefi-raw? If not I can add it to xtask, or just place this into uefi instead.

@andre-braga
Copy link
Contributor Author

Ah, I see the relevant section in api_guidelines now, I'll go about changing things

@andre-braga andre-braga marked this pull request as draft June 17, 2024 18:16
@andre-braga andre-braga marked this pull request as ready for review June 17, 2024 20:21
@phip1611
Copy link
Member

LGTM apart from #1211 (review)

@andre-braga andre-braga marked this pull request as draft June 20, 2024 13:04
@andre-braga andre-braga changed the title uefi-raw: Make TimeError more descriptive uefi: Make TimeError more descriptive Jun 20, 2024
@andre-braga andre-braga marked this pull request as ready for review June 20, 2024 13:18
@andre-braga
Copy link
Contributor Author

andre-braga commented Jun 20, 2024

I instead went with the struct method, allowing one to specify multiple errors.

Is this err == TimeError::default() idea that I've used idiomatic?

@phip1611
Copy link
Member

I instead went with the struct method, allowing one to specify multiple errors.

Is this err == TimeError::default() idea that I've used idiomatic?

We're almost there. Just add one more comment and we are good to go

When returning a TimeError, it'd be helpful to specify to the user which field
is invalid so that it can be handled accordingly, or at least communicated. This
change does this by adding bool fields to the TimeError struct, representing the
validity of each field of a Time struct.
@nicholasbishop nicholasbishop added this pull request to the merge queue Jun 20, 2024
@github-merge-queue github-merge-queue bot merged commit d7fa166 into rust-osdev:main Jun 20, 2024
12 checks passed
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.

3 participants