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

[Filesystem] mirror - fix copying content with same name as source/target. #23473

Closed
wants to merge 4 commits into
base: 2.7
from

Conversation

Projects
None yet
6 participants
@gitlost
Contributor

gitlost commented Jul 11, 2017

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

Uses substr() and lengths in Filesystem::mirror() rather than str_replace() to avoid multiple replacements.

@xabbuh

This comment has been minimized.

Show comment
Hide comment
@xabbuh

xabbuh Jul 12, 2017

Member

I would split the test case into two (one with the delete option, the other one without it) to make them independent from each other.

Member

xabbuh commented Jul 12, 2017

I would split the test case into two (one with the delete option, the other one without it) to make them independent from each other.

@nicolas-grekas

This comment has been minimized.

Show comment
Hide comment
@nicolas-grekas

nicolas-grekas Jul 28, 2017

Member

@gitlost could you please split the test case in two as suggested by @xabbuh ?

Member

nicolas-grekas commented Jul 28, 2017

@gitlost could you please split the test case in two as suggested by @xabbuh ?

@xabbuh

xabbuh approved these changes Aug 14, 2017

@xabbuh

This comment has been minimized.

Show comment
Hide comment
@xabbuh
Member

xabbuh commented Sep 13, 2017

@javiereguiluz

@gitlost thanks for fixing this bug ... and congrats on your first Symfony contribution!

@fabpot

fabpot approved these changes Sep 13, 2017

@fabpot

This comment has been minimized.

Show comment
Hide comment
@fabpot

fabpot Sep 13, 2017

Member

Thank you @gitlost.

Member

fabpot commented Sep 13, 2017

Thank you @gitlost.

fabpot added a commit that referenced this pull request Sep 13, 2017

bug #23473 [Filesystem] mirror - fix copying content with same name a…
…s source/target. (gitlost)

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

Discussion
----------

[Filesystem] mirror - fix copying content with same name as source/target.

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

Uses `substr()` and lengths in `Filesystem::mirror()` rather than `str_replace()` to avoid multiple replacements.

Commits
-------

b524c84 [Filesystem] mirror - fix copying content with same name as source/target.

@fabpot fabpot closed this Sep 13, 2017

This was referenced Oct 5, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment