Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Intl] fix wrong offset timezone PHP 8.1#45460

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
nicolas-grekas merged 1 commit intosymfony:4.4fromLenny4:FIX_RAW_OFFSET_TIMEZONES
Feb 18, 2022
Merged

[Intl] fix wrong offset timezone PHP 8.1#45460

nicolas-grekas merged 1 commit intosymfony:4.4fromLenny4:FIX_RAW_OFFSET_TIMEZONES
Feb 18, 2022

Conversation

@Lenny4
Copy link
Contributor

@Lenny4Lenny4 commentedFeb 17, 2022
edited by nicolas-grekas
Loading

QA
Branch?4.4
Bug fix?yes
New feature?no
Deprecations?no
TicketsFix#45451
LicenseMIT
Doc PR-

@carsonbot
Copy link

Hey!

I see that this is your first PR. That is great! Welcome!

Symfony has acontribution guide which I suggest you to read.

In short:

  • Always add tests
  • Keep backward compatibility (seehttps://symfony.com/bc).
  • Bug fixes must be submitted against the lowest maintained branch where they apply (seehttps://symfony.com/releases)
  • Features and deprecations must be submitted against the 6.1 branch.

Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change.

When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor!
If this PR is merged in a lower version branch, it will be merged up to all maintained branches within a few days.

I am going to sit back now and wait for the reviews.

Cheers!

Carsonbot

@carsonbotcarsonbot changed the titlefix: fix wrong offset timezone PHP 8.1[Intl] fix: fix wrong offset timezone PHP 8.1Feb 17, 2022
@nicolas-grekasnicolas-grekas changed the title[Intl] fix: fix wrong offset timezone PHP 8.1[Intl] fix wrong offset timezone PHP 8.1Feb 17, 2022
@nicolas-grekasnicolas-grekas modified the milestones:6.0,4.4Feb 17, 2022
@nicolas-grekasnicolas-grekas changed the base branch from6.0 to4.4February 17, 2022 14:04
Copy link
Member

@nicolas-grekasnicolas-grekas left a comment
edited
Loading

Choose a reason for hiding this comment

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

I pushed some changes, but I cannot reproduce the failure on my local setup.
When I revert the change, the new test case still passes. Can you confirm that's not the case for you?

@Lenny4
Copy link
ContributorAuthor

Lenny4 commentedFeb 17, 2022
edited
Loading

@nicolas-grekas
Which PHP version were you using ?

@nicolas-grekas
Copy link
Member

With both PHP 8.1 and 7.4

@nicolas-grekas
Copy link
Member

I ran the test case with
./phpunit src/Symfony/Component/Intl/ --group intl-data --filter testGetRawOffsetChangeTimeCountry

@Lenny4
Copy link
ContributorAuthor

This is really strange I got the same result as you.

@ro0NL
Copy link
Contributor

for me the patch works

docker run -it -v "$(pwd):/app" -w /app php:8.1 ./phpunit src/Symfony/Component/Intl/ --group intl-data --filter testGetRawOffsetChangeTimeCountry

@nicolas-grekas
Copy link
Member

nicolas-grekas commentedFeb 17, 2022
edited
Loading

@ro0NL the issue is not that tests are red, it's that they are always green, even when using PHP 8.1 BEFORE THE PATCH.

Lenny4 reacted with thumbs up emoji

Copy link
Contributor

@ro0NLro0NL left a comment

Choose a reason for hiding this comment

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

PHP 8.1 docker images uses:"Olson" Timezone Database Version => 2021.5

and the test is red without this patch 👍

Lenny4 reacted with heart emoji
@nicolas-grekas
Copy link
Member

Thank you@Lenny4.

@nicolas-grekasnicolas-grekas merged commit25ec51d intosymfony:4.4Feb 18, 2022
@fabpotfabpot mentioned this pull requestFeb 28, 2022
This was referencedFeb 28, 2022
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas left review comments

@lyrixxlyrixxAwaiting requested review from lyrixx

@ycerutoycerutoAwaiting requested review from yceruto

@wouterjwouterjAwaiting requested review from wouterj

@chalasrchalasrAwaiting requested review from chalasr

@dunglasdunglasAwaiting requested review from dunglas

@xabbuhxabbuhAwaiting requested review from xabbuh

+1 more reviewer

@ro0NLro0NLro0NL approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

4.4

Development

Successfully merging this pull request may close these issues.

4 participants

@Lenny4@carsonbot@nicolas-grekas@ro0NL

[8]ページ先頭

©2009-2025 Movatter.jp