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

Abstract logging logic to trait for non-FileTarget logging #4127

Closed
wants to merge 1 commit into from
Closed

Abstract logging logic to trait for non-FileTarget logging #4127

wants to merge 1 commit into from

Conversation

timkelty
Copy link
Contributor

This abstracts the non-FileTarget specific logic to a trait, and uses the init methods of FileTarget and LogTrait to set things dynamically (e.g. from Craft config).

This lets someone make a new logger with all the Craft-added stuff (message formatting, ip logging, etc) as simple as:

class MyTarget extends \yii\log\SyslogTarget
{
    use \craft\base\LogTrait;
}

@brandonkelly
Copy link
Member

I don’t like the idea of FileTarget configuring itself.

If you want to take advantage of the changes craft\log\FileTarget make over yii\Log\FileTarget, then just extend it.

@timkelty
Copy link
Contributor Author

timkelty commented Apr 11, 2019

I don’t like the idea of FileTarget configuring itself.

Fair enough, the LogTrait part was really the important part. I reverted the init parts if that is more agreeable.

If you want to take advantage of the changes craft\log\FileTarget make over yii\Log\FileTarget, then just extend it.

I can't, because craft\log\FileTarget already extends yii\Log\FileTarget, but I don't want to log to file.

What I'm after is a way to log using \yii\log\SyslogTarget or codemix\streamlog\Target, but with the Craft niceties that are currently stuck in craft\log\FileTarget, even though they aren't really specific to file logging.

Open to any alternatives.

@timkelty
Copy link
Contributor Author

@brandonkelly I rebased my branch on latest develop and removed the init configuring: develop...timkelty:log-trait

Apparently PRs don't get updated with new commits once they're closed.

If you still don't like this, let me know if you have other ideas (other than just copying craft\log\FileTarget wholesale into my own logger).

@brandonkelly
Copy link
Member

Ah, makes sense now. Just cleaned up some stuff and merged your log-trait branch. (fc54c0e)

@timkelty
Copy link
Contributor Author

Awesome, I appreciate it!

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