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

[Translation] Added logging capability to translator #10014

Closed
wants to merge 5 commits into from

Conversation

aitboudad
Copy link
Contributor

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #3015
License MIT

},
"suggest": {
"symfony/config": "",
"symfony/yaml": ""
"symfony/yaml": "",
"psr/log": "For using logging capability in translator"
Copy link

Choose a reason for hiding this comment

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

@aitboudad
psr/log contains interfaces only. You need to require some real implementation (most likely monolog/monolog) to have working logger

@inso
Copy link

inso commented Jan 15, 2014

Tested on my boxes and these changes add about 3% performance hit on page with ~260 translations:

# i7-3770
4.5958 ms vs 4.7394 ms (+0.1436 ms, +3.12%)
# Atom D525
25.9242 ms vs 26.6852 ms (+0.7607 ms, +2.93%)

Why not put all logging into LoggableTranslator and use it instead Translator based on 'kernel.debug' parameter?

@aitboudad
Copy link
Contributor Author

@fabpot What do you think ??
I'm 👍 for LoggableTranslator, I'll try to do that tomorrow.
thanks :).

@fabpot
Copy link
Member

fabpot commented Mar 27, 2014

@aitboudad Thanks for working on this. Unfortunately, translating methods being called a lot, we cannot add any overhead. Using composition to solve this problem (only in the dev env) is probably the best option (if possible). I'm closing this PR for now. If you want to work on a class that compose existing translator objects, feel free to give it a try and open a new PR.

@fabpot fabpot closed this Mar 27, 2014
@aitboudad aitboudad deleted the issue_3015 branch March 30, 2014 14:20
@aitboudad aitboudad restored the issue_3015 branch May 1, 2014 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants