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

remove duplicate calls to initObjectManager in bootstrap class #9140

Merged
merged 7 commits into from
Apr 12, 2017
Merged

remove duplicate calls to initObjectManager in bootstrap class #9140

merged 7 commits into from
Apr 12, 2017

Conversation

sivajik34
Copy link
Contributor

@sivajik34 sivajik34 commented Apr 5, 2017

There are duplicate calls for initobjectManager method,i think we can initialize objectmanager in constructor,so that object manager class will be available at the time of bootstrap object creation.

@magento-cicd2
Copy link
Contributor

magento-cicd2 commented Apr 5, 2017

CLA assistant check
All committers have signed the CLA.

@@ -279,7 +279,6 @@ protected function assertMaintenance()
if (null === $isExpected) {
return;
}
$this->initObjectManager();
/** @var \Magento\Framework\App\MaintenanceMode $maintenance */
$this->maintenance = $this->objectManager->get(\Magento\Framework\App\MaintenanceMode::class);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this could also be removed :) Maintenance is now also initialized in constructor.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Bartlomiejsz i think we can remove Maintenance initialization from constructor .
I don't know why Maintenance initialization happens in two times (assertMaintenance method,initObjectManager method)
i think Its enough in assertMaintenance.
anyway I'm looking for core team feedback?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sivajik34 @Bartlomiejsz To me it looks like it is enough only once. Seems to be some artifacts from legacy code. Going to look into it more deeply.

@ishakhsuvarov ishakhsuvarov self-assigned this Apr 11, 2017
@ishakhsuvarov ishakhsuvarov added this to the April 2017 milestone Apr 11, 2017
@magento-team magento-team merged commit 5834df6 into magento:develop Apr 12, 2017
@okorshenko
Copy link
Contributor

@sivajik34 thank you for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants