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

Init Kernel when possible during installation process #10296

Merged
merged 4 commits into from Oct 5, 2018

Conversation

Projects
None yet
7 participants
@Quetzacoalt91
Member

Quetzacoalt91 commented Sep 5, 2018

Questions Answers
Branch? 1.7.5.x
Description? Taken from @PierreRambaud's work. 9a7142d. Allow the Symfony container to be available during the installation as soon as the parameters file is created.
Type? bug fix
Category? IN
BC breaks? Nope
Deprecations? Nope
Fixed ticket? Fixes #10062
How to test? Retrieve the module ps_mbo v1.0.7, unzip it in the modules folder, and update the code to make it installed with the shop.

Installing a module natively

Add the technical module name in this list:

$modules = array(
'contactform',
'dashactivity',
'dashtrends',
'dashgoals',
'dashproducts',
'graphnvd3',
'gridhtml',
'gsitemap',
'ps_banner',
'ps_categorytree',
'ps_checkpayment',
'ps_contactinfo',
'ps_currencyselector',
'ps_customeraccountlinks',
'ps_customersignin',
'ps_customtext',
'ps_emailsubscription',
'ps_facetedsearch',
'ps_featuredproducts',
'ps_imageslider',
'ps_languageselector',
'ps_linklist',
'ps_mainmenu',
'ps_searchbar',
'ps_sharebuttons',
'ps_shoppingcart',
'ps_socialfollow',
'ps_themecusto',
'ps_wirepayment',
'pagesnotfound',
'sekeywords',
'statsbestcategories',
'statsbestcustomers',
'statsbestproducts',
'statsbestsuppliers',
'statsbestvouchers',
'statscarrier',
'statscatalog',
'statscheckup',
'statsdata',
'statsequipment',
'statsforecast',
'statslive',
'statsnewsletter',
'statsorigin',
'statspersonalinfos',
'statsproduct',
'statsregistrations',
'statssales',
'statssearch',
'statsstock',
'statsvisits',
'welcome',
);


This change is Reviewable

if (!is_null($kernel) && $kernel instanceof KernelInterface) {
if ($kernel === null) {
$kernel = new AppKernel(_PS_MODE_DEV_ ? 'dev' : 'prod', _PS_MODE_DEV_);

This comment has been minimized.

@tomlev

tomlev Sep 6, 2018

Member

is there any constant to define 'dev' and 'prod' ?

This comment has been minimized.

@PierreRambaud

PierreRambaud Sep 6, 2018

Contributor

of course not :trollface:

@jolelievre

This comment has been minimized.

Contributor

jolelievre commented Sep 17, 2018

what is still blocking this PR?

@Quetzacoalt91

This comment has been minimized.

Member

Quetzacoalt91 commented Sep 18, 2018

Well, we cannot go further on the review until I fix the Travis build. :)

@Quetzacoalt91 Quetzacoalt91 changed the title from Init Kernel when it does not exist while its container is requested to Init Kernel when possible during installation process Sep 20, 2018

@@ -82,6 +82,10 @@
require_once _PS_CORE_DIR_.'/config/autoload.php';
if (file_exists(_PS_CORE_DIR_.'/app/config/parameters.php')) {
require_once _PS_CORE_DIR_.'/config/bootstrap.php';
$kernel = new AppKernel(_PS_MODE_DEV_ ? 'dev' : 'prod', _PS_MODE_DEV_);

This comment has been minimized.

@PierreRambaud

PierreRambaud Sep 21, 2018

Contributor

Should you have to check for test env?

This comment has been minimized.

@Quetzacoalt91

Quetzacoalt91 Sep 21, 2018

Member

I don't know, is this case possible during installation?

This comment has been minimized.

@PierreRambaud

PierreRambaud Sep 21, 2018

Contributor

It's just to say to the kernel, "Please use var/cache/test instead of dev :P" and we will able to use specific services for test env.

@@ -150,7 +150,8 @@
"Sensio\\Bundle\\DistributionBundle\\Composer\\ScriptHandler::prepareDeploymentTarget"
],
"create-test-db": [
"Tests\\PrestaShopBundle\\Utils\\DatabaseCreator::createTestDB"

This comment has been minimized.

@tomlev

tomlev Sep 21, 2018

Member

why not keeping the static call since the script only uses the class ?

This comment has been minimized.

@Quetzacoalt91

Quetzacoalt91 Sep 21, 2018

Member

Calling a separate php process prevent a conflict between classes having the same name loaded in memory.

This comment has been minimized.

@Quetzacoalt91 Quetzacoalt91 changed the base branch from develop to 1.7.5.x Sep 21, 2018

@Quetzacoalt91 Quetzacoalt91 added this to the 1.7.5.0 milestone Sep 25, 2018

@PierreRambaud

This comment has been minimized.

Contributor

PierreRambaud commented Oct 5, 2018

@Quetzacoalt91 is it ok for you?

@Quetzacoalt91

This comment has been minimized.

Member

Quetzacoalt91 commented Oct 5, 2018

Yep!

@marionf marionf self-assigned this Oct 5, 2018

@marionf marionf added QA ✔️ and removed waiting for QA labels Oct 5, 2018

@marionf marionf removed their assignment Oct 5, 2018

@prestonBot prestonBot added the 1.7.5.x label Oct 5, 2018

@eternoendless eternoendless removed the develop label Oct 5, 2018

@eternoendless

This comment has been minimized.

Member

eternoendless commented Oct 5, 2018

@eternoendless eternoendless merged commit 9e1e121 into PrestaShop:1.7.5.x Oct 5, 2018

2 checks passed

Codacy/PR Quality Review Up to standards. A positive pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment