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

Add Period Start and End functions to Date and DateTime #3695

Merged
merged 18 commits into from
Sep 13, 2022

Conversation

radeusgd
Copy link
Member

@radeusgd radeusgd commented Sep 9, 2022

Pull Request Description

Implements https://www.pivotaltracker.com/story/show/183081152

Important Notes

Checklist

Please include the following checklist in your PR:

  • The documentation has been updated if necessary.
  • All code conforms to the
    Scala,
    Java,
    and
    Rust
    style guides.
  • All code has been tested:
    • Unit tests have been written where possible.
    • If GUI codebase was changed: Enso GUI was tested when built using BOTH
      ./run ide build and ./run ide watch.

@radeusgd radeusgd self-assigned this Sep 9, 2022
import java.time.temporal.Temporal;
import java.time.temporal.TemporalAdjusters;

public class Date_Period_Utils implements TimeUtilsBase {
Copy link
Member Author

Choose a reason for hiding this comment

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

@jdunkerley do you think I should add some doc comments? Seems a bit 'naked' without them but OTOH I'd say the name of the function seems to be pretty self explanatory so I'm not sure if the comments would add any clarity. What do you think?

Copy link
Contributor

@4e6 4e6 left a comment

Choose a reason for hiding this comment

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

Thanks for updating the SBT build formatting

@radeusgd radeusgd force-pushed the wip/radeusgd/date-period-start-end-183081152 branch from 41b8b52 to 3de8f31 Compare September 9, 2022 16:09
Copy link
Member

@jdunkerley jdunkerley left a comment

Choose a reason for hiding this comment

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

One minor rename and a suggestion.
But looks good.

@radeusgd
Copy link
Member Author

I could not fully test the autumn DST switch case as the overlapped timelines are not handled properly in polyglot which makes our Enso Date_Time unable to express one of the two instants in time there (2:30 am after the DST switch is not expressible, it gets converted back to 2:30 am before switch which is an hour earlier).

Reported this at oracle/graal#4918

@radeusgd radeusgd added CI: Ready to merge This PR is eligible for automatic merge and removed CI: Ready to merge This PR is eligible for automatic merge labels Sep 10, 2022
@hubertp hubertp self-requested a review September 12, 2022 08:05
@radeusgd radeusgd force-pushed the wip/radeusgd/date-period-start-end-183081152 branch from 39828dc to b728f05 Compare September 13, 2022 09:04
@radeusgd radeusgd added the CI: Ready to merge This PR is eligible for automatic merge label Sep 13, 2022
@mergify mergify bot merged commit b304402 into develop Sep 13, 2022
@mergify mergify bot deleted the wip/radeusgd/date-period-start-end-183081152 branch September 13, 2022 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: Ready to merge This PR is eligible for automatic merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants