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

[PropertyAccess] Fixed PropertyPathBuilder remove that fails to reset internal indexes #30392

Merged
merged 1 commit into from Mar 4, 2019
Merged

Conversation

GregOriol
Copy link
Contributor

@GregOriol GregOriol commented Feb 27, 2019

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

In addition to the fix (first commit), this PR adds \ to all not-yet-prefixed native functions in the PropertyPathBuilder (second commit).

NB: the behavior fixed here actually appeared in 2.2

@xabbuh
Copy link
Member

xabbuh commented Feb 27, 2019

Can you revert the second commit? We use the backslash only for a limited set of functions that are known to be optimised in this case.

@GregOriol
Copy link
Contributor Author

GregOriol commented Feb 27, 2019

@xabbuh Fixed

Also, I replace \array_splice (which shouldn't have the \ if I understood correctly?) with \array_slice (which does?), because it seems to be ~1.5x faster in this case

@fabpot
Copy link
Member

fabpot commented Mar 3, 2019

As this is a bug fix, it should target 3.4 instead of master.

@GregOriol GregOriol changed the base branch from master to 3.4 March 3, 2019 21:26
@GregOriol
Copy link
Contributor Author

Indeed, fixed! (I hope, sorry, only my 2nd little PR on this project)

@fabpot
Copy link
Member

fabpot commented Mar 4, 2019

Thank you @GregOriol.

@fabpot fabpot merged commit 479dff4 into symfony:3.4 Mar 4, 2019
fabpot added a commit that referenced this pull request Mar 4, 2019
…ls to reset internal indexes (GregOriol)

This PR was squashed before being merged into the 3.4 branch (closes #30392).

Discussion
----------

[PropertyAccess] Fixed PropertyPathBuilder remove that fails to reset internal indexes

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

In addition to the fix (first commit), this PR adds \ to all not-yet-prefixed native functions in the PropertyPathBuilder (second commit).

NB: the behavior fixed here actually appeared in 2.2

Commits
-------

479dff4 [PropertyAccess] Fixed PropertyPathBuilder remove that fails to reset internal indexes
This was referenced Apr 2, 2019
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.

None yet

4 participants