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

Made *TimeError exceptions subtypes of TimeError. #4

Merged
merged 2 commits into from
Aug 20, 2015
Merged

Conversation

omus
Copy link
Member

@omus omus commented Aug 13, 2015

Also moved the definitions before their first usage.

Also moved the definitions before their first usage.
@omus
Copy link
Member Author

omus commented Aug 13, 2015

Should I make PRs for minor stuff like this? I don't mind having a second set of eyes on this but for minor changes like this I'm comfortable pushing this directly to master if you're ok with it.

dt::DateTime
tz::TimeZone
end
Base.showerror(io::IO, e::AmbiguousTimeError) = print(io, "Local DateTime $(e.dt) is ambiguious");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is e.dt going to show the local datetime because this is thrown when trying to construct a ZonedDateTime? (just making sure there aren't other places where we'd through this where dt is UTC)

Copy link
Member Author

Choose a reason for hiding this comment

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

The DateTimes in these exceptions is just expected to be in localtime. We may want to extend the exceptions to be aware of what kind of DateTimes they contain (local/UTC). Most of the time you only get into trouble using local DateTimes which is why that is what is expected. There are some odd interactions within type.jl such as:

throw(NonExistentTimeError(localtime(zdt), tz))

@quinnj
Copy link
Collaborator

quinnj commented Aug 13, 2015

Definitely ok with you pushing stuff like this.

@omus
Copy link
Member Author

omus commented Aug 13, 2015

Sounds good. Anything controversial I'll make sure to generate a PR for.

omus added a commit that referenced this pull request Aug 20, 2015
Made *TimeError exceptions subtypes of TimeError.
@omus omus merged commit 2290f38 into master Aug 20, 2015
@omus omus deleted the timeerror branch August 20, 2015 18:44
omus referenced this pull request in invenia/TimeZones.jl Aug 15, 2018
omus referenced this pull request in invenia/TimeZones.jl Aug 15, 2018
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