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.

Sign up for GitHub

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

Jump to bottom

Logger is not always defined in Magento\Framework\Image\Adapter\Abstract... #893

Closed
wants to merge 1 commit into from
Closed

Logger is not always defined in Magento\Framework\Image\Adapter\Abstract... #893

wants to merge 1 commit into from

Conversation

pierredup
Copy link

...Adapter

I got this on a clean install on OSX with PHP 5.5.18:

Fatal error: Call to a member function critical() on a non-object in .../lib/internal/Magento/Framework/Image/Adapter/AbstractAdapter.php on line 678

@pierredup
Logger is not always defined in Magento\Framework\Image\Adapter\Abstr…
7475546 
…actAdapter

I got this on a clean install on OSX with PHP 5.5.18:

Fatal error: Call to a member function critical() on a non-object in .../lib/internal/Magento/Framework/Image/Adapter/AbstractAdapter.php on line 678
@maksek
Copy link
Contributor

maksek commented Jan 9, 2015

Hi @pierredup, Travis builds are red. Please review you PR.

@pierredup
Copy link
Author

Hi @maksek The failures are unrelated to this change

1) Magento\Framework\Data\Form\Element\DateTest::testGetValue with data set #0 (array('short', 'short', 1420221201), '01/2/15 9:53 AM')
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'01/2/15 9:53 AM'
+'1/2/15 9:53 AM'

/home/travis/build/magento/magento2/dev/tests/integration/testsuite/Magento/Framework/Data/Form/Element/DateTest.php:43

2) Magento\Framework\Data\Form\Element\DateTest::testGetValue with data set #2 (array('short', 1420221201), '01/2/15')
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'01/2/15'
+'1/2/15'

@maksek
Copy link
Contributor

maksek commented Jan 9, 2015

@pierredup, it supposed to be fixed with the commit - 1d7b54a
Or it is another issue?

@pierredup
Copy link
Author

That commit was added after the builds for this PR ran. So if you re-run the builds, then they should all pass

@maksek maksek self-assigned this Jan 14, 2015
@maksek maksek closed this Jan 14, 2015
@maksek maksek reopened this Jan 14, 2015
@mzeis
Copy link
Contributor

mzeis commented Jan 15, 2015

I got the same error, the patch bei @pierredup removed it.

@maksek maksek added Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development and removed Progress: needs update labels Jan 15, 2015
@maksek
Copy link
Contributor

maksek commented Jan 15, 2015

Hey @pierredup, i have re-run several times the builds, but travis is red. Can you please review and fix you PR.

antonkril
antonkril reviewed Jan 16, 2015
View reviewed changes
lib/internal/Magento/Framework/Image/Adapter/AbstractAdapter.php
@@ -675,7 +675,10 @@ protected function _prepareDestination($destination = null, $newName = null)
try {
$this->directoryWrite->create($this->directoryWrite->getRelativePath($destination));
} catch (\Magento\Framework\Filesystem\FilesystemException $e) {
$this->logger->critical($e);
if (null !== $this->logger) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't fix the cause of the issue.

Copy link

Choose a reason for hiding this comment

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

I suspect the bug is the logger property is not being initialized in constructor. Add it to constructor list and DI framework will provide it.

airbone42 pushed a commit to airbone42/magento2 that referenced this pull request Jan 25, 2015
magento#893 inject logger
1aac68a
@airbone42
Copy link

Looks like this was already fixed in latest dev ;)

@pierredup
Copy link
Author

Correct, It looks like the commit b2bcbba fixes the issue (In the proper way :) )

@pierredup pierredup closed this Jan 25, 2015
magento-team pushed a commit that referenced this pull request May 31, 2017
@buskamuza
Merge pull request #893 from magento-fearless-kiwis/FearlessKiwis-MAG…
0aabab8 
…ETWO-64401-TravisBuildFail-2.0

MAGETWO-64401: Travis Build Fail 2.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@maksek maksek

Labels
Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@pierredup @maksek @mzeis @airbone42 @antonkril @alankent