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

Fixed #88: Support combining W and D in intervals #85

Closed
wants to merge 1 commit into from

Conversation

gharlan
Copy link
Contributor

@gharlan gharlan commented Aug 4, 2020

https://bugs.php.net/bug.php?id=61366

With this PR "P1W2D" results in "9 days" instead of "2 days".

@gharlan
Copy link
Contributor Author

gharlan commented Aug 6, 2020

In ISO 8601-1:2019 it is not allowed to combine weeks with any other component.
But we already support combining weeks with other components, except with days. I think this is inconsistent.

And in ISO 8601-2:2019 it is allowed to combine weeks and other components, including days.

Copy link
Owner

@derickr derickr left a comment

Choose a reason for hiding this comment

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

Before I will consider this for merging, the following changes will have to be made:

  • There needs to be an issue in this repository, with title "Support combining W and D in intervals". (Don't refer to PHP tickets in the head line message, they can be in the description)
  • The commit messages needs to be in the form "Fixed #XX: Support combining W and D in intervals"
  • A test needs to be added to tests/c/all_tests.cpp with a new file tests/c/parse_intervals.cpp, that covers this new rule, but also a few other rules with W and D.

@gharlan gharlan changed the title intervals: support combining W and D (ticket 61366) Fixes #88: Support combining W and D in intervals Aug 9, 2020
@gharlan gharlan changed the title Fixes #88: Support combining W and D in intervals Fixed #88: Support combining W and D in intervals Aug 9, 2020
@gharlan gharlan requested a review from derickr August 9, 2020 14:07
derickr added a commit that referenced this pull request Aug 31, 2020
@derickr
Copy link
Owner

derickr commented Aug 31, 2020

This has been merged, and is part of the 2020.02 release, which made it into PHP 8.0 too. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants