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

Jump to bottom

Rename TimeZoneTransformer into TimezoneTransformer #21875

Closed
wants to merge 1 commit into from
Closed

Rename TimeZoneTransformer into TimezoneTransformer #21875

wants to merge 1 commit into from

Conversation

lepiaf
Copy link
Contributor

@lepiaf lepiaf commented Mar 5, 2017 •
edited

Q A
Branch? 3.2
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #11028
License MIT
Doc PR no

@javiereguiluz
Copy link
Member

@lepiaf thanks for contributing a fix for this super old issue report.

To give some context:

  1. Symfony uses Timezone sometimes and TimeZone others.
  2. PHP uses "TimeZone" in its classes: http://php.net/manual/en/class.intltimezone.php
  3. In UK English, "timezone" doesn't exist (it's "time zone") but in US English, it's more common to use "timezone", so the camelized version should be "Timezone".

Symfony always favors US English, so this change looks correct to me!

nicolas-grekas
nicolas-grekas reviewed Mar 6, 2017
View reviewed changes
UPGRADE-3.3.md Outdated
Intl
----

* Rename `TimeZoneTransformer` class into `TimezoneTransformer`
Copy link
Member

Choose a reason for hiding this comment

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

since the class is internal (since 2.8), I'd suggest removing these, and apply this PR on 3.2

Copy link
Member

Choose a reason for hiding this comment

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

LGTM

@nicolas-grekas nicolas-grekas added this to the 3.2 milestone Mar 6, 2017
@lepiaf lepiaf changed the base branch from master to 3.2 March 6, 2017 20:28
nicolas-grekas
nicolas-grekas reviewed Mar 9, 2017
View reviewed changes
UPGRADE-3.2.md Outdated
Intl
----

* Rename `TimeZoneTransformer` class into `TimezoneTransformer`
Copy link
Member

Choose a reason for hiding this comment

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

internal class, not needed

src/Symfony/Component/Intl/CHANGELOG.md Outdated
3.2.5
-----

* Rename `TimeZoneTransformer` class into `TimezoneTransformer`
Copy link
Member

Choose a reason for hiding this comment

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

internal class, not needed

nicolas-grekas
nicolas-grekas approved these changes Mar 14, 2017
View reviewed changes
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

👍

xabbuh
xabbuh approved these changes Mar 14, 2017
View reviewed changes
Copy link
Member

@xabbuh xabbuh left a comment

Choose a reason for hiding this comment

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

👍

@Tobion
Copy link
Member

Tobion commented Mar 14, 2017

👍

@fabpot
Copy link
Member

fabpot commented Mar 14, 2017

Good catch, thanks @lepiaf.

@fabpot fabpot closed this in dc0eba7 Mar 14, 2017
@nicolas-grekas nicolas-grekas mentioned this pull request Apr 4, 2017
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabpot fabpot fabpot left review comments

@nicolas-grekas nicolas-grekas nicolas-grekas approved these changes

@xabbuh xabbuh xabbuh approved these changes

Assignees
No one assigned
Labels
Form Status: Needs Review
Projects
None yet
Milestone
3.2
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@lepiaf @javiereguiluz @Tobion @fabpot @nicolas-grekas @xabbuh @carsonbot