Log level configuration for 2.0.x #29
Conversation
|
OK, I ran a few tests and it works well - thanks! I have added a few comments inline. Once you've fixed those up, can you please squash all your commits into a single one and force-push the feature-branch? Cheers :-) |
|
@phptek this is failing to build do to a silverstripe cms recipe not being available anymore. This tag seems to be two years old. Would you like me to patch that pipeline? |
|
Sure! Just needs to be the minimum version that composer will work with. |
|
I'm still not sure under what circumstances one might run this task. Can you explain? If it's still required, it should probably be restricted to running only with a permissioned user. |
|
@phptek this is not the task 🤔 |
|
@phptek could you rerun this pipeline? It should be ready to merge now :) |
There was a problem hiding this comment.
@phptek this is not the task
Not sure what you mean here.
This original PR was about adding a TestConnectionTask. I originally blindly looked at it using my phone, but now I'm looking at it from my lappy, I'm struggling to imagine how this task is used. What's the use case exactly?
src/Tasks/TestConnectionTask.php
Outdated
| foreach (Logger::getLevels() as $name => $value) { | ||
| $func = strtolower($name); | ||
| $logger->$func('Testing Severity Level: ' . $name); | ||
| printf("Security Level: %s <br/>\n", $name); |
There was a problem hiding this comment.
A <br> element is hard-coded here, but that won't suit CLI users (I very rarely use the browser to run BuildTasks). I usully include something like this in my tasks that spits out newlines suited to the current PHP SAPI:
public function output(string $message)
{
$newLine = Director::is_cli() ? PHP_EOL : '<br/>';
printf($message . $newLine);
}
src/Tasks/TestConnectionTask.php
Outdated
| protected $title = 'Test Sentry Configuration'; | ||
|
|
||
| protected $description = 'Captures message for all levels available'; |
There was a problem hiding this comment.
Minor point, but one I am fairly retentive on - docblocks. A single /** @var string */ to cover both works for me :-)
There was a problem hiding this comment.
FYI a single one won't cover both for tooling & IDEs ;)
src/Handler/SentryMonologHandler.php
Outdated
| $log_level = Config::inst()->get(self::class, 'log_level'); | ||
|
|
||
| $level = ($log_level) ? constant(Logger::class . '::'. $log_level) : $level; |
There was a problem hiding this comment.
Minor point, but there's no need for a newline here.
|
@phptek got confused between PRs. This PR is both the |
Allos override of minimum reporting level Add Task that tests Sentry Link Change config parameter name Specific comment for differences in versions 2 & 3 Sentry Testing Task Add all emergency levels available to options Change Namespace to correct one Default `log_level` set to `WARNING` Add output with new line wrapper. Removes unnecessary new lines.
Added configuration for minimal log level config on 2.0.x
Parameter name is the same as what has been implemented for 3.0.x
Update README accordingly.