-
Notifications
You must be signed in to change notification settings - Fork 326
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
Add Period
type
#3818
Add Period
type
#3818
Conversation
0967404
to
bfa068f
Compare
distribution/lib/Standard/Base/0.0.0-dev/src/Data/Time/Date.enso
Outdated
Show resolved
Hide resolved
distribution/lib/Standard/Base/0.0.0-dev/src/Data/Time/Period.enso
Outdated
Show resolved
Hide resolved
distribution/lib/Standard/Base/0.0.0-dev/src/Data/Time/Date_Time.enso
Outdated
Show resolved
Hide resolved
distribution/lib/Standard/Base/0.0.0-dev/src/Data/Time/Date.enso
Outdated
Show resolved
Hide resolved
distribution/lib/Standard/Base/0.0.0-dev/src/Data/Time/Duration.enso
Outdated
Show resolved
Hide resolved
distribution/lib/Standard/Base/0.0.0-dev/src/Data/Time/Period.enso
Outdated
Show resolved
Hide resolved
distribution/lib/Standard/Base/0.0.0-dev/src/Data/Time/Period.enso
Outdated
Show resolved
Hide resolved
distribution/lib/Standard/Base/0.0.0-dev/src/Data/Time/Date_Time.enso
Outdated
Show resolved
Hide resolved
8e572f9
to
96ab675
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor nits, looks OK otherwise
distribution/lib/Standard/Base/0.0.0-dev/src/Data/Time/Period.enso
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fine from my "engine" perspective.
5db7413
to
edf289e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
ba6ff98
to
79ee0b4
Compare
Date.+ accepts only Period
79ee0b4
to
2e516da
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally looks good but am not sure about the ==
and compare_to
- adding the starting date feels wrong to me.
distribution/lib/Standard/Base/0.0.0-dev/src/Data/Time/Period.enso
Outdated
Show resolved
Hide resolved
It caused stack overflow, probabaly because ensure_in_epoch breaks tail call.
distribution/lib/Standard/Base/0.0.0-dev/src/Data/Time/Duration.enso
Outdated
Show resolved
Hide resolved
LGTM - will want to add these types to default Base exports but will do as part of the great tidy up... |
Pull Request Description
This PR adds
Period
type, which is a date-only complement toDuration
builtin type.Important Notes
Period
replacesDate_Period
, andTime_Period
.Duration
andPeriod
. For example:Period.days 10
instead ofPeriod.new days=10
.Period
can be compared to otherPeriod
in some cases, other cases throw an error.Checklist
Please include the following checklist in your PR:
Scala,
Java,
and
Rust
style guides.
./run ide build
and./run ide watch
.