Removed deprecated log levels and fixed inconsistent use of LevelFactory #578
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of the change
1. Removed deprecated code from
Level
The first major change removes some deprecated constants and the
__callStatic()
from theRollbar\Payload\Level
class. These have been deprecated sincev1.2.0
, and it seems like now is a good time to remove them. This means that, although deprecated, aLevel
instance could be constructed by callingLevel::ERROR()
. This was very concise but lacked any type annotations or consistency. TheLevelTest.php
file shows this.2. Refactored
LevelFactory
with static methodsThe second major change is changing the public
LevelFactory
methods to be static and replacing theLevelFactory::init()
method with theLevelFactory::getLevels()
method. This change eliminates the need to instantiateLevelFactory
before calling any methods. Creating a valid newLevel
instance is more verbose than I like. Making the methods static reduces the total code needed to validate and create a newLevel
. See the examples below.Perhaps in a PHP 8.1 and above world we can use a backed
enum
and a few methods withmatch
expressions to eliminate the need for theLevelFactory
class entirely.3. Replaced instances of
LevelFactory
with static method callsThe third significant change is removing the
$levelFactory
property from theRollbar\Config
,Rollbar\DataBuilder
, andRollbar\RollbarLogger
classes. Since theLevelFactory
methods can now be called statically, we no longer need instances ofLevelFactory
easily accessible on these classes.This has the possibility to introduce a change in behavior. It was possible for someone to provide their own level factory class when instantiating
DataBuilder
. However, I don't think this would be likely or work very well sinceRollbarLogger
attempts to validate the level using ourLevelFactory
class before it gets toDataBuilder
.With only partial support for a custom level factory theoretically available this feels like something that should be cleaned up. We should use one level factory across the codebase. Either provided by the user or our own. Support for a custom level factory is undocumented, partial, and probably just a bad idea anyway. Because of that I have completely removed it.
Type of change
Related issues
None.
Checklists
Development
Code review