[GH-ISSUE #825] 2.0 refactor attempt #318

New issue

Closed

opened 2026-03-04 02:14:05 +03:00 by kerem · 3 comments

kerem commented

2026-03-04 02:14:05 +03:00

Owner

Originally created by @denys-potapov on GitHub (Jul 20, 2016).
Original GitHub issue: https://github.com/Seldaek/monolog/issues/825

I've made an attempt to refactor the Monolog library https://github.com/denys-potapov/monolog-oc/tree/oc:

Reorganise the Special Handlers and add GeneralHandler that joins bubble and ProcessableHandlerTrait.
Make the Logger a subclass of GroupHandler
Split the new logger API NewLogger and backward compatible Logger
Wrap the LogLevel
Extract special Date and Time behaviour to DateTimeProcessor

The whole library stays mainly backward compatible. If you find some ideas worse implementing, I can prepare a more cleaner separated pull request (with strict types and proper tests).

Originally created by @denys-potapov on GitHub (Jul 20, 2016). Original GitHub issue: https://github.com/Seldaek/monolog/issues/825 I've made an attempt to refactor the Monolog library [https://github.com/denys-potapov/monolog-oc/tree/oc](https://github.com/denys-potapov/monolog-oc/tree/oc): 1. Reorganise the Special Handlers and add [GeneralHandler](https://github.com/denys-potapov/monolog-oc/blob/oc/src/Monolog/Handler/GeneralHandler.php) that joins bubble and ProcessableHandlerTrait. 2. Make the Logger a subclass of GroupHandler 3. Split the new logger API [NewLogger](https://github.com/denys-potapov/monolog-oc/blob/oc/src/Monolog/NewLogger.php) and backward compatible [Logger](https://github.com/denys-potapov/monolog-oc/blob/oc/src/Monolog/Logger.php) 4. Wrap the [LogLevel](https://github.com/denys-potapov/monolog-oc/blob/oc/src/Monolog/LogLevel.php) 5. Extract special Date and Time behaviour to [DateTimeProcessor](https://github.com/denys-potapov/monolog-oc/blob/oc/src/Monolog/Processor/DateTimeProcessor.php) The whole library stays mainly backward compatible. If you find some ideas worse implementing, I can prepare a more cleaner separated pull request (with strict types and proper tests).

kerem closed this issue

2026-03-04 02:14:05 +03:00

kerem commented

2026-03-04 02:14:06 +03:00

Author

Owner

@hkdobrev commented on GitHub (Aug 28, 2016):

Have you seen #197?

@hkdobrev commented on GitHub (Aug 28, 2016): Have you seen #197?

kerem commented

2026-03-04 02:14:06 +03:00

Author

Owner

@denys-potapov commented on GitHub (Sep 1, 2016):

@hkdobrev, yes I've seen it. But my code contains many changes, and probably only some of them would be useful in monolog master, so I decided to make a separate issue.

@denys-potapov commented on GitHub (Sep 1, 2016): @hkdobrev, yes I've seen it. But my code contains many changes, and probably only some of them would be useful in monolog master, so I decided to make a separate issue.

kerem commented

2026-03-04 02:14:06 +03:00

Author

Owner

@Seldaek commented on GitHub (Sep 25, 2016):

Hey.. finally I had a look at this, sorry for the delay!

I am not sure what this brings compared to the current traits? At least with the traits the handlers don't have processor/formatter-related stuff when they don't need them.
(and 3) I don't really like this.. It gets confusing IMO if you nest loggers as you have a log record that enters one, gets a channel, then goes on to a nested logger and it gets another channel suddenly. It also means the logger class has a bunch of unrelated methods (handle/handleBatch & co are kind of "internal" and most users shouldn't need to know about or see them). It makes IDE autocompletion messy.
See above
Again, what does this bring us? It just adds layers of object orientation, same as the ProcessorStack.. OO is nice and all but non-OO isn't necessarily bad.
That's an interesting idea but also kinda confusing for users that suddenly a processor appears that they didn't configure. And if they popProcessor once too many then suddenly no more dates on the log records. Plus in master branch right now the date stuff was already extracted into Monolog\DateTimeImmutable which groups most things. Sure it still uses two instance variables instead of one, but I don't see this as a killer sorry.

So yeah overall not so willing to go with these changes, I see that they are interesting from an academic perspective and probably a good exercise in refactoring, but not clear enough to me what the value is for users of the lib.

@Seldaek commented on GitHub (Sep 25, 2016): Hey.. finally I had a look at this, sorry for the delay! 1. I am not sure what this brings compared to the current traits? At least with the traits the handlers don't have processor/formatter-related stuff when they don't need them. 2. (and 3) I don't really like this.. It gets confusing IMO if you nest loggers as you have a log record that enters one, gets a channel, then goes on to a nested logger and it gets another channel suddenly. It also means the logger class has a bunch of unrelated methods (handle/handleBatch & co are kind of "internal" and most users shouldn't need to know about or see them). It makes IDE autocompletion messy. 3. See above 4. Again, what does this bring us? It just adds layers of object orientation, same as the ProcessorStack.. OO is nice and all but non-OO isn't necessarily bad. 5. That's an interesting idea but also kinda confusing for users that suddenly a processor appears that they didn't configure. And if they popProcessor once too many then suddenly no more dates on the log records. Plus in master branch right now the date stuff was already extracted into `Monolog\DateTimeImmutable` which groups most things. Sure it still uses two instance variables instead of one, but I don't see this as a killer sorry. So yeah overall not so willing to go with these changes, I see that they are interesting from an academic perspective and probably a good exercise in refactoring, but not clear enough to me what the value is for users of the lib.