Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

Fix ChromePhp logger interface and debug level #3987

Closed
wants to merge 3 commits into from

Conversation

xoob
Copy link
Contributor

@xoob xoob commented Mar 7, 2013

The ChromePhp log writer logs debug messages as error messages. It looks like the interface was copied from FirePhp and not updated.

This pull request changes the ChromePhpInterface to match the actual class and fixes the log level used for debug messages.

*
* @param string $line
*/
public function error($line);
public function log($line);
Copy link
Contributor

Choose a reason for hiding this comment

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

We can only change that interface for ZF3. Can you find another solution?

Copy link
Member

Choose a reason for hiding this comment

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

@prolic Actually, it appears that this is a matter of just moving things around; we already have a log method in the interface, and @xoob appears to have swapped the log and error method placements in the interface.

@ghost ghost assigned weierophinney Mar 8, 2013
*
* @param string $line
*/
public function groupEnd($line);
Copy link
Member

Choose a reason for hiding this comment

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

That said... @xoob -- additions to an interface are not backwards compatible, nor is removing a method (I see you've removed trace and added group). Which is problematic, as I'm not sure how we can expose these features otherwise.

My guess is nobody else will implement the interface; they'll likely only ever use the writer we provide. Let me bring this to the community review team to discuss.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@weierophinney Understood, makes perfect sense. I'm not sure why the interface defines a trace() method though, but I suspect it is by mistake.

The actual ChromePhp class that is called defines this:

public static function getInstance()
public static function log()
public static function warn()
public static function error()
public static function group()
public static function info()
public static function groupCollapsed()
public static function groupEnd()
public function addSetting($key, $value)
public function addSettings(array $settings)
public function getSetting($key)

@xoob
Copy link
Contributor Author

xoob commented Mar 12, 2013

Reverted the original fix and made another one that just changes the error() call to log(). Long-term I think it's still a good idea to update the ChromePhpInterface to match the actual ChromePhp class though.

weierophinney added a commit that referenced this pull request Mar 12, 2013
Fix ChromePhp logger interface and debug level
weierophinney added a commit that referenced this pull request Mar 12, 2013
weierophinney added a commit to zendframework/zend-log that referenced this pull request May 15, 2015
…chromephp

Fix ChromePhp logger interface and debug level
weierophinney added a commit to zendframework/zend-log that referenced this pull request May 15, 2015
weierophinney added a commit to zendframework/zend-log that referenced this pull request May 15, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants