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

span: improve invalid units error message #91

Merged
merged 1 commit into from
Aug 11, 2024
Merged

Conversation

BurntSushi
Copy link
Owner

Previously, we would report a range error when trying to use units of
days or greater in a span when adding to a Timestamp. But this error
doesn't really do a good job of explaining anything. It was a hold-over
from the days where I was using many different error types and it was
difficult to improve messages on an ad hoc basis. But now, we can just
write a message.

The error message doesn't explain why days-and-greater are banned, but
it's hard to fit all of that into an error. At some point, we just have
to rely on folks reading the docs.

Closes #90

Previously, we would report a range error when trying to use units of
days or greater in a span when adding to a `Timestamp`. But this error
doesn't really do a good job of explaining anything. It was a hold-over
from the days where I was using many different error types and it was
difficult to improve messages on an ad hoc basis. But now, we can just
write a message.

The error message doesn't explain *why* days-and-greater are banned, but
it's hard to fit all of that into an error. At some point, we just have
to rely on folks reading the docs.

Closes #90
@BurntSushi BurntSushi merged commit c659069 into master Aug 11, 2024
17 checks passed
@BurntSushi BurntSushi deleted the ag/unit-ban-error branch August 11, 2024 12:34
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.

Timestamp adds 1 day panic
1 participant