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

simple logging implementation #95

Closed
wants to merge 5 commits into from

Conversation

Projects
None yet
2 participants
@tomasstrejcek
Copy link
Contributor

commented Dec 18, 2015

solves #94

why not something as simple as this? I presume you would do it yourself if it was about something easy as this, so whats wrong with solutions? Depending on Traxy\ILogger ?

p.s.: It would definitely benefit from implementing config for logging, but i didnt have time for that :)

@@ -130,6 +137,7 @@ public function translate($message, $count = NULL, $parameters = array(), $domai
return $message;
} elseif ($message instanceof Nette\Utils\Html) {
$this->logger->log($message, static::LOGGER_NAMESPACE);

This comment has been minimized.

Copy link
@fprochazka

fprochazka Jan 9, 2016

Member

Are you sure it makes sense to log this?

This comment has been minimized.

Copy link
@tomasstrejcek

tomasstrejcek Jan 10, 2016

Author Contributor

definitely not :)

but the following code marks message as untraslated, so I presumed logging in this place is also desired..

This comment has been minimized.

Copy link
@fprochazka

fprochazka Jan 10, 2016

Member

Well, it makes sense to inform the developer on the page, but there might be cases where this is intended and it would just spam the log.

@@ -79,12 +85,13 @@ class Translator extends BaseTranslator implements ITranslator
* @param IResourceLoader $loader
*/
public function __construct(IUserLocaleResolver $localeResolver, MessageSelector $selector,
CatalogueCompiler $catalogueCompiler, FallbackResolver $fallbackResolver, IResourceLoader $loader)
CatalogueCompiler $catalogueCompiler, FallbackResolver $fallbackResolver, IResourceLoader $loader, ILogger $logger)

This comment has been minimized.

Copy link
@fprochazka

fprochazka Jan 9, 2016

Member

I'd preffer optional PSR logger

This comment has been minimized.

Copy link
@tomasstrejcek

tomasstrejcek Jan 10, 2016

Author Contributor

Well, you know, this is Nette extension, sou Tracy shouldn't be such a burden

Could it implement both and on the app level decide which it should use with priority on LoggerInterface instead of ILogger ?

Missing message would then end up in warning loglevel?

@tomasstrejcek

This comment has been minimized.

Copy link
Contributor Author

commented Jan 10, 2016

maybe the tests should be expanded by psr/log?

@fprochazka

This comment has been minimized.

Copy link
Member

commented Jan 17, 2016

Thank you for your work @tomasstrejcek, let's continue here #97

@fprochazka fprochazka closed this Jan 17, 2016

@tomasstrejcek

This comment has been minimized.

Copy link
Contributor Author

commented Jan 18, 2016

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.