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

Added support for PSR-3 compliant logger. #89

Closed

Conversation

ManInTheBox
Copy link
Contributor

This PR adds ability to use PSR-3 compliant logger alongside JAXL's logging service implementation.

JAXL logging implementation is not modified, and logging levels are properly mapped to the ones defined by PSR-3.

Feel free to suggest better design implementation than the one I presented here.

Thanks!

Signed-off-by: Zarko Stankovic <stankovic.zarko@gmail.com>

/**
* @requires PHP 5.4
* @requires function uopz_backup
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is probably not needed. This test was failing for me because debug_backtrace() doesn't produce expected output by the code. I think that code needs refactoring in order for these test to be useful.

Copy link

@umpirsky umpirsky left a comment

Choose a reason for hiding this comment

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

Looks good. 👍

@@ -137,4 +153,24 @@ public static function setColors(array $colors)
self::$colors[$k] = $v;
}
}

/**
* @param \Psr\Log\LoggerInterface $psr3Logger

Choose a reason for hiding this comment

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

Remove this, it's redundant.

}

/**
* @return \Psr\Log\LoggerInterface

Choose a reason for hiding this comment

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

@return LoggerInterface is enough.

@@ -122,8 +124,9 @@ class JAXL extends XMPPStream

/**
* @param array $config
* @param LoggerInterface $psr3Logger

Choose a reason for hiding this comment

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

This is redundant and misleading. And incorrect, since null is allowed.

@@ -154,6 +157,10 @@ public function __construct(array $config)
JAXLLogger::$level = $this->log_level = $this->cfg['log_level'];
JAXLLogger::$colorize = $this->log_colorize = $this->cfg['log_colorize'];

if ($psr3Logger !== null) {

Choose a reason for hiding this comment

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

Alternatively we can allow null in setPsr3Logger() and remove if.

@abhinavsingh
Copy link
Member

@ManInTheBox are we awaiting any further changes suggested by @umpirsky? If not I'll merge this in. Thank You!!!

@ManInTheBox
Copy link
Contributor Author

ManInTheBox commented Dec 3, 2018

Thanks @umpirsky and @abhinavsingh! I'll update the code and let you know when it's ready for merging.

@ManInTheBox
Copy link
Contributor Author

This is really outdated. Discarding. Feel free to reopen if needed.

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.

3 participants