Replace < with <= in NonISODateAdd around month boundaries #98
Conversation
Uplifts unicode-org/icu4x#7257 Caused by spec bug: tc39/proposal-intl-era-monthcode#98 Change-Id: I5df814a7ef7c2c8b1e44affd34fd31d017c04b28 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7201147 Auto-Submit: Manish Goregaokar <manishearth@google.com> Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Commit-Queue: Daniel Cheng <dcheng@chromium.org> Cr-Commit-Position: refs/heads/main@{#1549444}
sffc
left a comment
sffc
left a comment
There was a problem hiding this comment.
I recall thinking that this condition didn't matter for < vs <= because the resulting day is the same either way, but then I think the overflow reject got grafted in a bit later and made it matter whether < or <= is used. Thanks for the catch
ptomato
left a comment
ptomato
left a comment
There was a problem hiding this comment.
I had been hoping to write a test to verify this similar to what Ben and I have been doing for #101. But I don't think I'll have time and I don't want to be the bottleneck here. LGTM with that caveat.
Note that there is identical language in NonISODateSurpasses, but there it really does not matter because the "If overflow is REJECT" step is not present. We could change to ≤ there just for consistency, or leave it as-is. I don't have a strong opinion.
Good catch. I favor consistency between those two AOs. |
Uplifts unicode-org/icu4x#7257 Caused by spec bug: tc39/proposal-intl-era-monthcode#98 Change-Id: I5df814a7ef7c2c8b1e44affd34fd31d017c04b28 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7201147 Auto-Submit: Manish Goregaokar <manishearth@google.com> Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Commit-Queue: Daniel Cheng <dcheng@chromium.org> Cr-Commit-Position: refs/heads/main@{#1549444} NOKEYCHECK=True GitOrigin-RevId: 02da9f75c4cb912e2dd0e0fdad60803d207f06aa
3 participants
endOfMonth.Day is a valid day value, we should not be erroring for it.