On Monday, 14 October 2013 at 11:39:52 UTC, Dicebot wrote:
As `std.logger` is still marked as "work in progress" this
thread is less formal that typical pre-voting review. Goal is
to provide as much input about desirable `std.logger`
functionality and current state and let module author to use
this information for preparing proposal for actual review /
voting.
Lets unleash the forces of constructive destruction.
===== Meta =====
Code:
https://github.com/D-Programming-Language/phobos/pull/1500/files
Docs:
http://burner.github.io/phobos/phobos-prerelease/std_logger.html
First announcement / discussion thread :
http://forum.dlang.org/post/mailman.313.1377180809.1719.digitalmar...@puremagic.com
Ok, finally making some conclusions.
===================
As a review manager
===================
This what needs to be done before moving to next review stage:
1.
Judging by review comments it is clear that some more efforts
need to be put into demonstrating how existing platform can be
used as a standard API for more complex logging functionality.
Addition of `MultiLogger` is good thing here, but providing
either separate article on topic or extra module that
demonstrates enhancing existing system feels like a good way to
avoid confusion.
That said, I do agree that std.logger itself should not be "all
batteries included" module as it does contradict overall Phobos
style. It may be extended in future once we start using
sub-packages much more but right now having good standard API is
much more important.
2.
Standard logger must be made thread-safe by default. Out of the
box safety is very important for standard library.
3.
Making default `log` a conditional logger does not seem to be
gladly accepted decision. I'd recommend to swap it with `logf`
and rename to `logIf`.
Naming overall should be reviewed to make sure there is no word
duplication in typical usage pattern - Phobos does favor plain
procedural/functional approach as default and D module system
makes it unnecessary to encode namespaces into names.
====================
Personal preferences
====================
It is a matter of personal taste but any single thing in that
list will make me vote "No" on logger proposal:
1.
API it too verbose.
I want stuff as short and simple as this:
```
import std.logger;
warn("1");
inform("2");
die("3");
```
I don't want to refer logger class instances explicitly for any
typical task.
Stuff like `auto logger = new MultiLogger();
logger.insertLogger(someOtherLogger);` is redundant beyond absurd
- just how many times one may insert word "logger" in a single
sentence? :)
2.
Logging payload needs to include fully qualified module name.
This is necessary for enabling module-local output.
3.
I am not sure if proper global vs local log separation can be
done without providing two distinct default instances in
std.logger (as I have already said, I don't consider solutions
which imply tracking class instance manually).
4.
Static "namespace" classes are redundant and should be replaced
with module-scope globals.
