On 10/14/2013 02:39 PM, Sönke Ludwig wrote: > Am 14.10.2013 13:39, schrieb Dicebot: >> 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 >> >> > > Sorry in advance for the long list of issues. I think the general > approach is fine, but over the years I've grown some preferences for > this stuff. Generally, I think we should make sure that such a module > is flexible enough to fulfill most people's needs, or it will probably > fail the widespread adoption that is desired to actually improve > interoperability. > > - LogLevel: enum values should start lower case according to the > Phobos conventions. will be fixed > - The static methods in LogManager should be made global and the class > be removed. It's not for objects so it shouldn't be a class. LogManager also stores the global log level. Sure I can make another static global function storing this log level, but I would like to keep them together as they belong together IMO. > - For me this logger is completely worthless without any debug log > levels. The last std.log entry had at least anonymous verbosity > levels, but I'd prefer something like I did in vibe.d [1], where > each level has a defined role. This should especially improve the > situation when multiple libraries are involved. Logger.log(LogLevel.(d|D)ebug, "Your message"); > - Similarly, I've never felt the need for conditional logging, but > without it being lazy evaluated what's the use for it, actually? The conditional logging part is totally transparent. > - I really think there should be shortcuts for the different log > levels. Typing out "LogLevel.xxx" each time looks tedious for > something that is used in so many places. One could argue that writting logger.logDebug("...") is more tedious than writing, logger.logLevel = LogLevel.xxx; logger.log("..."); logger.log("..."); ...
This has been argued in the last logger discussion to some extend and it looked to me like this is the mostly preferred version. > - There should be some kind of MultiLogger so that multiple log > destinations can be configured (e.g. console + file). Or, instead of > a single default logger, there could be a list of loggers. there is one default logger. I will create a MultiLogger, good point. I'm currently sure how to store the multi logger (LL or Array or ... ) > - Logger.logMessage: Exchanging the bunch of parameters with a single > struct that is passed by reference makes the API much more > flexible/future-proof and is more efficient when the data needs to > be passed on to other functions. good point > - "shared" - probably best to leave this out until we have a verified > design for that - but in theory the loggers should probably be > shared. my thoughts exactly > - On the same topic, if I'm right and the default logger is stored as > __gshared, it should be documented that Loggers need to be > thread-safe. It is not stored __gshared, but If, you're right. > - GC allocations for each log message _must_ be avoided at all costs. > Using formattedWrite() instead of format() on either a temporary > buffer that is reused, or, better, on some kind of output range > interface of the Logger would solve this. This was proposed in the last thread. A fixed size buffer would scream bufferoverflow, a dynamic buffer not but both would raise the question of thread safety. > - A note on DDOC comments: The first paragraph of a doc comment is > treated as a short description according to the DDOC spec. It should > be kept to around a single sentence, followed by a more detailed > paragraph. While this doesn't matter much with the current HTML doc > layout, the short description is used in the single-page > documentation inside of overview tables [2]. Will be fixed Awesome comments, thanks