Skip to content

[HttpFoundation] Removed compatibility layer for PHP <5.4 sessions #24338

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

Merged

Conversation

afurculita
Copy link
Contributor

@afurculita afurculita commented Sep 26, 2017

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

This is a follow-up of #24239. This PR removes the compatibility layer added for sessions for PHP <5.4.

@afurculita afurculita changed the title Removed compatibility layer for PHP <5.4 sessions [HttpFoundation] Removed compatibility layer for PHP <5.4 sessions Sep 26, 2017
@afurculita afurculita force-pushed the remove-session-handler-proxies branch from 875696b to 241566f Compare September 26, 2017 19:32
@@ -47,9 +47,8 @@ public function testEnvironment()

public function testGetSession()
{
$session = $this->getMockBuilder(Session::class)->disableOriginalConstructor()->getMock();
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this changed again?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because of the fact that the session is constructed using an instance of NativeSessionStorage that by default used the deprecated SessionHandlerProxy, this test did not pass due to the deprecation error. Now there are no deprecations, so no need for disabling the original constructor,

Copy link
Contributor

Choose a reason for hiding this comment

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

We should be using MockArraySessionStorage rather than NativeSessionStorage in tests (or create a mock as before).

Copy link
Member

@chalasr chalasr left a comment

Choose a reason for hiding this comment

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

CHANGELOG should be updated to mention removals

Signed-off-by: Alexandru Furculita <alex@furculita.net>
@afurculita afurculita force-pushed the remove-session-handler-proxies branch from 241566f to 37d1a21 Compare September 27, 2017 08:58
@afurculita
Copy link
Contributor Author

Changelog updated

@afurculita
Copy link
Contributor Author

AppVeyor failure not related to this PR

@fabpot
Copy link
Member

fabpot commented Sep 27, 2017

Thank you @afurculita.

@fabpot fabpot merged commit 37d1a21 into symfony:master Sep 27, 2017
fabpot added a commit that referenced this pull request Sep 27, 2017
…5.4 sessions (afurculita)

This PR was merged into the 4.0-dev branch.

Discussion
----------

[HttpFoundation] Removed compatibility layer for PHP <5.4 sessions

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?  | yes
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

This is a follow-up of #24239. This PR removes the compatibility layer added for sessions for PHP <5.4.

Commits
-------

37d1a21 Removed compatibility layer for PHP <5.4 sessions
xabbuh added a commit to symfony/symfony-docs that referenced this pull request Oct 13, 2017
This PR was merged into the master branch.

Discussion
----------

Update outdated session documentation with SF 4

The PHP 5.3 compatibilty and proxy logic has been removed in symfony/symfony#24338
The MemcacheSessionHandler is removed with symfony/symfony#24447

Commits
-------

bd765c1 Update outdated session documentation with SF 4
@fabpot fabpot mentioned this pull request Oct 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants