Skip to content

Conversation

@nicolas-grekas
Copy link
Member

@nicolas-grekas nicolas-grekas commented Apr 28, 2016

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

throw new InvalidArgumentException(sprintf('RedisAdapter namespace contains "%s" but only characters in [-+_.A-Za-z0-9] are allowed.', $match[0]));
}
$this->redis = $redisConnection;
$this->redis->setOption(\Redis::OPT_PREFIX, $namespace);
Copy link
Member

Choose a reason for hiding this comment

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

this breaks things if the same Redis connection is reused for several things, as you change its state

Copy link
Member Author

Choose a reason for hiding this comment

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

true, reverted

foreach ($ids as $id) {
if (false !== $value = $values[$index++]) {
$result[$id] = unserialize($value);
$result = array();
Copy link
Contributor

Choose a reason for hiding this comment

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

As it is for Sf 3.1 brackets were great, isn't it ?

Copy link
Member

Choose a reason for hiding this comment

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

@Nek- our policy is to not use them, to keep consistency in the codebase (and we cannot convert the whole codebase to the short array syntax, as we will need to merge 2.8 into 3.x during 3 years, and we don't want git conflicts each time we change a line dealing with an array)

@fabpot
Copy link
Member

fabpot commented Apr 28, 2016

Thank you @nicolas-grekas.

@fabpot fabpot merged commit 1165459 into symfony:master Apr 28, 2016
fabpot added a commit that referenced this pull request Apr 28, 2016
…as-grekas)

This PR was merged into the 3.1-dev branch.

Discussion
----------

[Cache] Dont use Redis connection when not required

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

Commits
-------

1165459 [Cache] Dont use Redis connection when not required
@nicolas-grekas nicolas-grekas deleted the cache-redis branch April 28, 2016 12:09
@fabpot fabpot mentioned this pull request May 13, 2016
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.

5 participants