Skip to content

Conversation

inso
Copy link

@inso inso commented Aug 14, 2015

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets n/a
License MIT
Doc PR n/a

Reworked version of #15537

Difference with original PR:

  • sort $names array to ensure its order does not affect caching, added test for it
  • minor tweaks for cache key format
  • used foreach instead of array_map

P.S. I'm not proficient with tests so not sure if tests are correct.

@sstok
Copy link
Contributor

sstok commented Aug 14, 2015

LGTM.

@inso inso changed the title [expression-language] Fixed expressions cache key generation [ExpressionLanguage] Fixed expressions cache key generation Aug 14, 2015
@inso
Copy link
Author

inso commented Sep 6, 2015

ping @symfony/deciders

@Tobion
Copy link
Contributor

Tobion commented Sep 6, 2015

👍

Status: Reviewed

@fabpot
Copy link
Member

fabpot commented Sep 7, 2015

Thank you @inso.

fabpot added a commit that referenced this pull request Sep 7, 2015
…n (inso)

This PR was squashed before being merged into the 2.7 branch (closes #15552).

Discussion
----------

[ExpressionLanguage] Fixed expressions cache key generation

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

Reworked version of #15537

Difference with original PR:
- sort $names array to ensure its order does not affect caching, added test for it
- minor tweaks for cache key format
- used `foreach` instead of `array_map`

P.S. I'm not proficient with tests so not sure if tests are correct.

Commits
-------

4114a2b [ExpressionLanguage] Fixed expressions cache key generation
@fabpot fabpot closed this Sep 7, 2015
@inso inso deleted the expression-language-cache-key branch May 24, 2016 21:05
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