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

StreamLogger #283

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

StreamLogger #283

wants to merge 2 commits into from

Conversation

JanTvrdik
Copy link
Contributor

  • bug fix? no
  • new feature? yes
  • BC break? no

implements #280

@dg
Copy link
Member

dg commented Feb 22, 2018

Isn't better this

Debugger::setLogger(new StreamLogger);
Debugger::enable();

than this?

Debugger::enable(Debugger::DETECT, new StreamLogger);

@JanTvrdik
Copy link
Contributor Author

The issue is that in Nette you're never actually calling Debugger::enable(), because there is $configurator->enableDebugger(). So, the actual comparison is

Tracy\Debugger::setLogger(new Tracy\StreamLogger('php://stderr'));

$configurator = new Nette\Configurator();
$configurator->enableDebugger();

vs.

$configurator = new Nette\Configurator();
$configurator->enableDebugger(new Tracy\StreamLogger('php://stderr'));

@JanTvrdik
Copy link
Contributor Author

When I think about it now, the logic could be moved to Nette\Configurator – but it would create disparity between Tracy\Debuger::enable() and Nette\Configurator::enableDebugger() signatures.

@dg
Copy link
Member

dg commented Feb 25, 2018

create disparity between

It does not matter.

@dg dg force-pushed the master branch 4 times, most recently from d13a9ce to 7f24887 Compare February 25, 2018 23:58
@dg dg force-pushed the master branch 8 times, most recently from 427177a to 7ac33a2 Compare March 26, 2018 11:48
@dg dg force-pushed the master branch 11 times, most recently from 8675719 to 51fdf8a Compare April 6, 2018 06:37
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.

2 participants