Improve detection of Time-Zone inconsistencies #2456
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
smortex
force-pushed
the
tz-handling-improvement
branch
4 times, most recently
from
9b5715e
to
3bb4414
Compare
smortex
requested review from
elfisher,
AMoo-Miki,
nknize,
krisfreedain,
peterzhuamazon,
CEHENKLE,
dtaivpp,
kolchfa-aws and
nateynateynate
as code owners
|
This will hopefully avoid more work for @krisfreedain and @nateynateynate to review and merge my PRs to fix Time-Zone issues 😅 Cc @joshuarrrr who also hit some Time-Zone related issue. I tried to split each part in a separate commit to make it easier to follow the fix process. As a bonus, we can see that the check script report less and less failures as we go. Thanks! |
|
WOW! Thank you @smortex!! @astephanus - could you give this a look? |
We have a track record of Time-Zone inconsistencies in the events of the project website (e.g. opensearch-project#1616, opensearch-project#1805, opensearch-project#2288, opensearch-project#2418, opensearch-project#2422, opensearch-project#2431, opensearch-project#2449). In order to detect these inconsistencies earlier, make the `tz` mandatory and check that the Time-Zone offset match the Time-Zone name. These checks require a working bundle so run it on GitHub actions and keep the old pre-commit check to only check date formatting using basic UNIX tooling. Adjust the events templates to match what we expect. While here, also check that non-online events have the needed `location.city` and `location.country`. Signed-off-by: Romain Tartière <[email protected]>
Most of these events have a Time-Zone offset of -0700 or -0800 which makes me think they are all using the America/Los_Angeles Time-Zone (sometimes with the wrong offset, which will be fixed in another commit). Updated with: ``` grep -r --files-without-match '^tz:' _events | xargs sed -i '' -E -e'/^online:/a\ tz: America/Los_Angeles' ``` Signed-off-by: Romain Tartière <[email protected]>
These inconsistencies are now being reported, when -0700 and -0800 are used incorectly. Signed-off-by: Romain Tartière <[email protected]>
According to the meetup page, the event was on 24.01.2023 at 19:00 local time. The Time-Zone offset then correspond to Europe/Warsaw. Signed-off-by: Romain Tartière <[email protected]>
The website says it started on the 25th and not the 24th in Charlottesville, same Time-Zone as New York (America/Charlottesville not being a valid Time-Zone name). Signed-off-by: Romain Tartière <[email protected]>
These one have the correct offset, but when adding `tz` we did not took this into account. Use the America/New_York offset which match the events. Signed-off-by: Romain Tartière <[email protected]>
These are not RFC822 time representations. So do not call them that name, it just add confusion. Signed-off-by: Romain Tartière <[email protected]>
Signed-off-by: Romain Tartière <[email protected]>
smortex
force-pushed
the
tz-handling-improvement
branch
from
16a96f8
to
eb12673
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We have a track record of Time-Zone inconsistencies in the events of the
project website (e.g. #1616, #1805, #2288, #2418, #2422, #2431, #2449).
In order to detect these inconsistencies earlier, make the
tzmandatory and check that the Time-Zone offset match the Time-Zone name.
These checks require a working bundle so run it on GitHub actions and
keep the old pre-commit check to only check date formatting using basic
UNIX tooling.
Adjust the events templates to match what we expect.
While here, also check that non-online events have the needed
location.cityandlocation.country.