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

Fix Interval#count counting the endpoint as part of the interval #1308

Merged

Conversation

diesieben07
Copy link
Collaborator

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.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Oct 19, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: diesieben07 / name: Take Weiland (1cc8497)

@diesieben07 diesieben07 force-pushed the issue/1306/interval-length-endpoint branch from 1cc8497 to 19c7421 Compare October 19, 2022 12:10
@icambron
Copy link
Member

@diesieben07 Just a quick note to say I'm not ignoring this, just super slammed at the moment

@icambron icambron merged commit 33f7957 into moment:master Mar 4, 2023
@nerdstep
Copy link

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.

@diesieben07
Copy link
Collaborator Author

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.

@nerdstep
Copy link

@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!

@diesieben07
Copy link
Collaborator Author

I agree, we should work on improving the changelog for sure. Thank you for your feedback.

CarsonF added a commit to SeedCompany/cord-api-v3 that referenced this pull request Apr 4, 2023
brboi added a commit to odoo-dev/odoo that referenced this pull request Aug 30, 2023
**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
robodoo pushed a commit to odoo/odoo that referenced this pull request Sep 4, 2023
**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) <luvi@odoo.com>
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.

Interval#count incorrectly includes the end-point
3 participants