Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[DI] Enhance logging in compiler passes#21396

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
fabpot merged 1 commit intosymfony:masterfromnicolas-grekas:di-logs
Jan 30, 2017

Conversation

@nicolas-grekas
Copy link
Member

QA
Branch?master
Bug fix?no
New feature?yes
BC breaks?no
Deprecations?yes
Tests pass?yes
Fixed tickets-
LicenseMIT
Doc PR-

We should log more in compiler passes - and this should be better integrated in usual log reports.

For logging more, let's drop LoggingFormatter and add a simple "log" method on ContainerBuilder.
For better integration, let's throw silenced notices - they can be caught by our Debug handler.

linaori, theofidry, robfrawley, and chalasr reacted with thumbs up emoji
@nicolas-grekas
Copy link
MemberAuthor

nicolas-grekas commentedJan 24, 2017
edited
Loading

It would be great to have a profiler panel that displays the contents of the var/cache/dev/appDevDebugProjectContainerCompiler.log file btw!
Would someone like to give it a try?

linaori, theofidry, robfrawley, and Nicofuma reacted with thumbs up emoji

@nicolas-grekasnicolas-grekasforce-pushed thedi-logs branch 2 times, most recently froma7f3d74 to9f0545eCompareJanuary 24, 2017 21:17
*/
publicfunctionaddLogMessage($string)
{
@trigger_error($string);
Copy link
Member

Choose a reason for hiding this comment

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

this will trigger them asE_USER_ERROR. IMO, it is too high. What aboutE_USER_NOTICE instead ? Lower than notice would be even better as this is a debugging info, but trigger_error does not have something lower (as it is not meant to be used as a debug logger)

btw, I'm not even convinced this is a good idea (at least not today as people will complain about silenced notices in the profiler)

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

looking at the phdoc, the default isE_USER_NOTICE, so all good.
but I now made it explicit, and configurable (via a third "$level" arg)

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

hum OK, I removed the trigger_error call for now - but kept the $level arg: even unused, it make the signature compatible with future changes requiring it.

@stof
Copy link
Member

The panel showing the container logs is a good idea though

@nicolas-grekasnicolas-grekasforce-pushed thedi-logs branch 3 times, most recently froma0623c9 to03be6ddCompareJanuary 25, 2017 13:46
$this->getCompiler()->log($pass,sprintf('Removed service "%s"; reason: %s.',$id,$reason));
}

publicfunctionlog(CompilerPassInterface$pass,$message,$level =E_USER_NOTICE)
Copy link
Member

Choose a reason for hiding this comment

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

I don't think passing a level here makes much sense, especially not as a PHP error level. It would couple it too much to the error triggering layer, while we talk about logging here.
If you want to provide a logging level, use PSR-3 levels instead.

Btw, a way to allow flexibility about adding a level only the day we need it is to make the method final: final method can add optional args as they want (adding an optional arg only breaks BC for inheritance)

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

good idea, they are final now

@nicolas-grekasnicolas-grekasforce-pushed thedi-logs branch 3 times, most recently from70b9a6e to21b8b34CompareJanuary 26, 2017 07:19
@nicolas-grekas
Copy link
MemberAuthor

"logRemoveService" removed

@fabpot
Copy link
Member

Thank you@nicolas-grekas.

@fabpotfabpot merged commitfb200a0 intosymfony:masterJan 30, 2017
fabpot added a commit that referenced this pull requestJan 30, 2017
This PR was merged into the 3.3-dev branch.Discussion----------[DI] Enhance logging in compiler passes| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | yes| BC breaks?    | no| Deprecations? | yes| Tests pass?   | yes| Fixed tickets | -| License       | MIT| Doc PR        | -We should log more in compiler passes - and this should be better integrated in usual log reports.For logging more, let's drop LoggingFormatter and add a simple "log" method on ContainerBuilder.For better integration, let's throw silenced notices - they can be caught by our Debug handler.Commits-------fb200a0 [DI] Enhance logging in compiler passes
@nicolas-grekasnicolas-grekas deleted the di-logs branchJanuary 30, 2017 20:27
@fabpotfabpot mentioned this pull requestMay 1, 2017
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@stofstofstof left review comments

Assignees

No one assigned

Projects

None yet

Milestone

3.3

Development

Successfully merging this pull request may close these issues.

4 participants

@nicolas-grekas@stof@fabpot@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp