Skip to content

Add to config minimumLevel option. #440

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.

Sign up for GitHub

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

Closed
Yozhef wants to merge 3 commits into from
Closed

Add to config minimumLevel option. #440

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
218bdd9
Add to config minimumLevel
Mar 12, 2019
b6b2666
Fixed test, and add default value for new config minimumLevel
Mar 13, 2019
f480fa9
Change value name to monologMinimumLevel
Mar 22, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/Config.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ class Config
'include_raw_request_body',
'local_vars_dump',
'max_nesting_depth',
'max_items'
'max_items',
'minimum_level'
Copy link
Contributor

@ArturMoczulski ArturMoczulski Mar 17, 2019

Choose a reason for hiding this comment

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

I believe this option would only be used with the Monolog handler. I suggest changing this config option to monolog_minimum_level. What do you think?

Copy link
Contributor Author

@Yozhef Yozhef Mar 18, 2019

Choose a reason for hiding this comment

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

I think is better because this params is found and maybe people use this. You have a good test how it's work. And very long name monolog_minimum_level.

Copy link
Contributor

@ArturMoczulski ArturMoczulski Mar 21, 2019

Choose a reason for hiding this comment

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

My concern is with the possible confusion of what this config option might be. This is something that will be only used with Monolog handlers. If we don't make that crystal clear a lot of users might mistakenly assume the config option will apply even if they don't use Monolog. Does it make sense?

Copy link
Contributor Author

@Yozhef Yozhef Mar 22, 2019

Choose a reason for hiding this comment

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

@ArturMoczulski I thought and still reworked on your offer. =)

);

private $accessToken;
Expand Down
8 changes: 7 additions & 1 deletion src/Defaults.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?php namespace Rollbar;

use Rollbar\Utilities;
use Monolog\Logger;
use Rollbar\Payload\Notifier;
use Psr\Log\LogLevel;

Expand Down Expand Up @@ -313,6 +313,11 @@ public function maxItems($value = null)
return $value !== null ? $value : $this->maxItems;
}

public function minimumLevel($value = null)
{
return $value !== null ? $value : $this->minimumLevel;
}

private $psrLevels;
private $errorLevels;
private $autodetectBranch = false;
Expand Down Expand Up @@ -358,4 +363,5 @@ public function maxItems($value = null)
private $scrubFields;
private $customTruncation = null;
private $maxItems = 10;
private $minimumLevel = Logger::ERROR;
}