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

Initialize container in WebService environment (bis) #14899

Merged
merged 8 commits into from Aug 1, 2019

Conversation

@Progi1984
Copy link
Contributor

commented Jul 29, 2019

Questions Answers
Branch? develop
Description? Initialize container in WebService environment
Type? bug fix
Category? WS
BC breaks? yes
Deprecations? no
Fixed ticket? Fixes #14842
How to test? * Try the script given in issue #14842#issuecomment-514593911

This change is Reviewable

@Progi1984 Progi1984 requested a review from PrestaShop/prestashop-core-developers as a code owner Jul 29, 2019

@PierreRambaud
Copy link
Contributor

left a comment

See comments

@Progi1984 Progi1984 requested a review from PierreRambaud Jul 29, 2019

src/Adapter/ContainerFinder.php Show resolved Hide resolved
src/Adapter/ContainerFinder.php Show resolved Hide resolved
tests/Unit/Adapter/ContainerFinderTest.php Outdated Show resolved Hide resolved
tests/Unit/Adapter/ContainerFinderTest.php Outdated Show resolved Hide resolved
tests/Unit/Adapter/ContainerFinderTest.php Outdated Show resolved Hide resolved
tests/Unit/Adapter/ContainerFinderTest.php Outdated Show resolved Hide resolved
src/Adapter/ContainerFinder.php Outdated Show resolved Hide resolved
src/Adapter/ContainerFinder.php Outdated Show resolved Hide resolved
src/Adapter/ContainerFinder.php Outdated Show resolved Hide resolved
@mickaelandrieu
Copy link
Contributor

left a comment

good job 👍

src/Adapter/ContainerFinder.php Outdated Show resolved Hide resolved
src/Adapter/ContainerFinder.php Outdated Show resolved Hide resolved
src/Adapter/ContainerFinder.php Outdated Show resolved Hide resolved
@jolelievre
Copy link
Contributor

left a comment

Nice approach for the ContainerBuilder but the ContainerFinder seems a bit weird

classes/Tools.php Outdated Show resolved Hide resolved

@Progi1984 Progi1984 dismissed stale reviews from mickaelandrieu and PierreRambaud via 7de04c2 Jul 30, 2019

webservice/dispatcher.php Show resolved Hide resolved
classes/Tools.php Outdated Show resolved Hide resolved

@Progi1984 Progi1984 requested a review from eternoendless Jul 31, 2019

@eternoendless

This comment has been minimized.

Copy link
Member

commented Jul 31, 2019

Looking good!

@khouloudbelguith

This comment has been minimized.

Copy link
Contributor

commented Aug 1, 2019

Hi @Progi1984,

It is OK.

Thanks!

@khouloudbelguith khouloudbelguith added this to the 1.7.6.1 milestone Aug 1, 2019

@marionf marionf modified the milestones: 1.7.6.1, 1.7.7.0 Aug 1, 2019

@marionf marionf added develop and removed 1.7.6.x labels Aug 1, 2019

@marionf

This comment has been minimized.

Copy link
Contributor

commented Aug 1, 2019

Question guys, seems the issue is a regression, this PR shouldn't be on 1.7.6.x branch ?

@PierreRambaud PierreRambaud merged commit 585a71b into PrestaShop:develop Aug 1, 2019

2 checks passed

PrettyCI Code formatting
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@PierreRambaud

This comment has been minimized.

Copy link
Contributor

commented Aug 1, 2019

Thanks @Progi1984

@marionf

This comment has been minimized.

Copy link
Contributor

commented Aug 1, 2019

@Progi1984 @PierreRambaud
This PR should have been done on 176x branch

@matthieu-rolland

This comment has been minimized.

Copy link
Contributor

commented Aug 1, 2019

Thanx @Progi1984 !

I think @marionf is right, I needed this on 1.7.6.x for the double translation service issue :D

Maybe I should implement the same logic on 176x in my translation fixing branch ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
10 participants
You can’t perform that action at this time.