Skip to content
This repository has been archived by the owner on Jul 4, 2018. It is now read-only.

Fixed Session Service Provider registry #1343

Merged
merged 1 commit into from May 4, 2016

Conversation

ragboyjr
Copy link
Contributor

@ragboyjr ragboyjr commented May 4, 2016

The service provider was registering the wrong parameters
to the session listener instead of the actual session class

This fixes #1342

Signed-off-by: RJ Garcia rj@bighead.net


/**
* Test that you can properly configure the Session class.
*/
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need this comment.

@ragboyjr
Copy link
Contributor Author

ragboyjr commented May 4, 2016

@fabpot. Those are updated.

out of curiosity with doing conditionals on one line, how would you format a bunch of conditions so that the line isn't so long.

$long_var_name == 1 || $long_var_name_b == 1 || ($long_var_name_2 == 2 || $long_var_name3 == 2) && $one_more_long_var_name ==5 

@fabpot
Copy link
Member

fabpot commented May 4, 2016

Everyone has a large screen nowadays, so there is no need to cut lines at 80 chars.

$session = $app['session'];

$valid = $session->getBag('flashes') === $flash && $session->getBag('attributes') === $attrs;
$this->assertTrue($valid);
Copy link
Member

Choose a reason for hiding this comment

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

I'd have be more precise like:

$this->assertSame($flash, $session->getBag('flashes');
$this->assertSame($attrs, $session->getBag('attributes');

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No problem, I was just trying to keep the whole 1 assertion per test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated.

The service provider was registering the wrong parameters
to the session listener instead of the actual session class

Signed-off-by: RJ Garcia <rj@bighead.net>
@fabpot
Copy link
Member

fabpot commented May 4, 2016

Thank you @ragboyjr.

@fabpot fabpot merged commit cecdb85 into silexphp:master May 4, 2016
fabpot added a commit that referenced this pull request May 4, 2016
This PR was merged into the 2.0.x-dev branch.

Discussion
----------

Fixed Session Service Provider registry

The service provider was registering the wrong parameters
to the session listener instead of the actual session class

This fixes #1342

Signed-off-by: RJ Garcia <rj@bighead.net>

Commits
-------

cecdb85 Fixed Session Service Provider registry
@ragboyjr ragboyjr deleted the session-provider-params branch May 8, 2016 07:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Something seams wrong in SessionServiceProvider
2 participants