-
Notifications
You must be signed in to change notification settings - Fork 742
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
Fix Interval#count counting the endpoint as part of the interval #1308
Fix Interval#count counting the endpoint as part of the interval #1308
Conversation
|
1cc8497
to
19c7421
Compare
@diesieben07 Just a quick note to say I'm not ignoring this, just super slammed at the moment |
Just a head's up, this was a breaking change for us -- we had dealt with what we thought was a feature by subtracting 1 from the count. Not a huge deal since we caught it in unit tests, but I can imagine that this could potentially break other folks too. |
Sorry about that. There is always a fine line between "do we consider this a bug fix, or is this a breaking change". You don't want to go down the PHP route of maintaining stupid bugs like a decade later, because "it might break people's code", but you also don't want to intentionally break people's code. |
@diesieben07 it's all good, and I'm with you on that, I'd rather see a bug fixed than codified as an ugly wart. My only suggestion would perhaps be a stronger callout in the release notes. Either way, I certainly still appreciate the work and contributions towards maintaining this excellent library! |
I agree, we should work on improving the changelog for sure. Thank you for your feedback. |
**Changelog** **3.4.2 (2023-08-26)** - Fixes regression from 3.4.1 (moment/luxon#1493) **3.4.1 (2023-08-23)** - Fixes for regressions from 3.4.0 (moment/luxon#1482 and moment/luxon#1488) **3.4.0 (2023-08-08)** - Fix type checking on input zones - Fix Islamic months listing - Fix normalize() for negative inputs **3.3.0 (2023-03-03)** - Fix off-by-one in Interval#count (moment/luxon#1308) - Support formatting for custom zones (moment/luxon#1377) - Fix parsing for narrow spaces (moment/luxon#1369) - Handle leap year issue with AD 100 (moment/luxon#1390) - Allow parsing of just an offset **3.2.1 (2023-01-04)** - Fix for RFC-2822 regex vulnerability - Better handling of BCP tags with -x- extensions **3.2.0 (2022-12-29)** - Allow timeZone to be specified as an intl option - Fix for diff's handling of end-of-month when crossing leap years (moment/luxon#1340) - Add Interval.toLocaleString() (moment/luxon#1320) **3.1.1 (2022-11-28)** - Add Settings.twoDigitCutoffYear **3.1.0 (2022-10-31)** - Add Duration.rescale **3.0.4 (2022-09-24)** - Fix quarters in diffs (moment/luxon#1279) - Export package.json in package (moment/luxon#1239) **3.0.2 (2022-08-28)** - Lots of doc changes - Added DateTime.expandFormat - Added support for custom conversion matrices in Durations
**Changelog** **3.4.2 (2023-08-26)** - Fixes regression from 3.4.1 (moment/luxon#1493) **3.4.1 (2023-08-23)** - Fixes for regressions from 3.4.0 (moment/luxon#1482 and moment/luxon#1488) **3.4.0 (2023-08-08)** - Fix type checking on input zones - Fix Islamic months listing - Fix normalize() for negative inputs **3.3.0 (2023-03-03)** - Fix off-by-one in Interval#count (moment/luxon#1308) - Support formatting for custom zones (moment/luxon#1377) - Fix parsing for narrow spaces (moment/luxon#1369) - Handle leap year issue with AD 100 (moment/luxon#1390) - Allow parsing of just an offset **3.2.1 (2023-01-04)** - Fix for RFC-2822 regex vulnerability - Better handling of BCP tags with -x- extensions **3.2.0 (2022-12-29)** - Allow timeZone to be specified as an intl option - Fix for diff's handling of end-of-month when crossing leap years (moment/luxon#1340) - Add Interval.toLocaleString() (moment/luxon#1320) **3.1.1 (2022-11-28)** - Add Settings.twoDigitCutoffYear **3.1.0 (2022-10-31)** - Add Duration.rescale **3.0.4 (2022-09-24)** - Fix quarters in diffs (moment/luxon#1279) - Export package.json in package (moment/luxon#1239) **3.0.2 (2022-08-28)** - Lots of doc changes - Added DateTime.expandFormat - Added support for custom conversion matrices in Durations closes #133599 Related: odoo/enterprise#46556 Signed-off-by: Luca Vitali (luvi) <[email protected]>
Fixes #1306
One of the tests actually tested this incorrect behavior, which I have adjusted. I have also added a new test, explicitly testing the correct behavior.