View mode: basic / threaded / horizontal-split · Log in · Help
March 03, 2012
Re: Review of Jose Armando Garcia Sancio's std.log
On 3/3/12 5:51 AM, Sönke Ludwig wrote:
> Am 17.02.2012 21:48, schrieb Andrei Alexandrescu:
>> On 2/17/12 12:06 PM, Jose Armando Garcia wrote:
>>> info("%s message", Severity.info);
>>
>> I think defaulting to formatted stuff may be confusing.
>>
>> Andrei
>
> Do you often log static messages? In my experience the majority of
> logging calls contains dynamic information and using a format string is
> the most convenient and readable form (compared to concatenating using
> multiple arguments).
>
> An additional infof(), warnf() etc. would be an acceptable alternative,
> although not very pretty. But if you'd always have to write
> info.format() or something similar, then this would be a real turn-off,
> at least for me.

I think it's fine to default to formatted. Also, it just occurred to me 
that we can adjust the formatting primitives to leave any "%"s alone if 
there are no matching arguments. Consider:

log.info("100% done!");

This is technically an error, but since D has static knowledge there are 
no variadics, it could simply output the string. That's faster, too, 
because no validation of the string is necessary.

Thoughts?


Andrei
March 04, 2012
Re: Review of Jose Armando Garcia Sancio's std.log
On 04.03.2012 2:17, Andrei Alexandrescu wrote:
> On 3/3/12 5:51 AM, Sönke Ludwig wrote:
>> Am 17.02.2012 21:48, schrieb Andrei Alexandrescu:
>>> On 2/17/12 12:06 PM, Jose Armando Garcia wrote:
>>>> info("%s message", Severity.info);
>>>
>>> I think defaulting to formatted stuff may be confusing.
>>>
>>> Andrei
>>
>> Do you often log static messages? In my experience the majority of
>> logging calls contains dynamic information and using a format string is
>> the most convenient and readable form (compared to concatenating using
>> multiple arguments).
>>
>> An additional infof(), warnf() etc. would be an acceptable alternative,
>> although not very pretty. But if you'd always have to write
>> info.format() or something similar, then this would be a real turn-off,
>> at least for me.
>
> I think it's fine to default to formatted. Also, it just occurred to me
> that we can adjust the formatting primitives to leave any "%"s alone if
> there are no matching arguments. Consider:
>
> log.info("100% done!");
>
> This is technically an error, but since D has static knowledge there are
> no variadics, it could simply output the string. That's faster, too,
> because no validation of the string is necessary.
>
> Thoughts?

I would say just separate them please. Let it work as writeln/writefln 
and nobody is hurt.

With all manner of magic rules the innocent will catch a bullet sooner 
or latter. And it's always happens kind of point-blank, when the code is 
shipped & forgotten.
Turning on imagination, e.g. for rarely used stuff:
log.warn("100%done for task %s, yet the client timed out", taskName);


-- 
Dmitry Olshansky
March 04, 2012
Re: Review of Jose Armando Garcia Sancio's std.log
On 3/4/12 3:17 AM, Dmitry Olshansky wrote:
> I would say just separate them please. Let it work as writeln/writefln
> and nobody is hurt.
>
> With all manner of magic rules the innocent will catch a bullet sooner
> or latter. And it's always happens kind of point-blank, when the code is
> shipped & forgotten.
> Turning on imagination, e.g. for rarely used stuff:
> log.warn("100%done for task %s, yet the client timed out", taskName);

Yah, agreed. Probably calls with no extra arguments should be safe.

Andrei
March 05, 2012
Re: Review of Jose Armando Garcia Sancio's std.log
On Mon, 13 Feb 2012 10:50:04 -0500, David Nadlinger <see@klickverbot.at>  
wrote:

> There are several modules in the review queue right now, and to get  
> things going, I have volunteered to manage the review of Jose's std.log  
> proposal. Barring any objections, the review period starts now and ends  
> in three weeks, on March 6th, followed by a week of voting.


Some notes:

I dislike that logging affects function execution.  In particular, I don't  
think the logging library should have any business throwing exceptions or  
errors.  It should be invisible to the application.  The equivalent  
function can be had by giving a wrapper function (i.e. log this message at  
the fatal level, and then throw an error).  A use case I can see is  
printing several fatal log messages before exiting.

The log aliases use names that are too common.  I think log.info is a  
better symbol for logging than just 'info', which could be a symbol in a  
myriad of places.  Given that D's symbol lookup rules allow shadowing of  
global symbols, this does not work out very well.

Like others have stated, I think vlog is a) confusing, and b)  
unnecessary.  Even reading the docs, I can't understand what it's used  
for, and why it has such different syntax than the normal logging stuff.

I really like the every function, that's a great idea, one that I've  
manually implemented (at least the once every N times) many times.

Do we have to make the logger a singleton?  I'd like to see cases where I  
can have different log instances.  For example, an instance I can  
enable/disable per class type, or an instance that logs to a diffferent  
backend.  Or a non-shared instance which does not need to handle threading  
issues (i.e. a per-thread file log).  Does this help with the vlog issue?

-Steve
March 05, 2012
Re: Review of Jose Armando Garcia Sancio's std.log
On Monday, 5 March 2012 at 21:55:08 UTC, Steven Schveighoffer 
wrote:
> The log aliases use names that are too common.  I think 
> log.info is a better symbol for logging than just 'info', which 
> could be a symbol in a myriad of places.  Given that D's symbol 
> lookup rules allow shadowing of global symbols, this does not 
> work out very well.

Originally, the code used log!info and so on, but it was changed 
to the current design right after review begin, the rationale 
being that you could always use »import log = std.log« if you 
want the extra namespace.

> Like others have stated, I think vlog is a) confusing, and b) 
> unnecessary.  Even reading the docs, I can't understand what 
> it's used for, and why it has such different syntax than the 
> normal logging stuff.

I think this been modelled after glog's verbose logging support 
[1], just like much of the rest of the design (by the way, I 
think a note about this should added somewhere in the module 
docs). Does the feature as described in the glog docs make sense 
to you?

> I really like the every function, that's a great idea, one that 
> I've manually implemented (at least the once every N times) 
> many times.

I love it too, a similar design served me really well in some 
larger *shudder* ActionScript projects.

David


[1] 
http://google-glog.googlecode.com/svn/trunk/doc/glog.html#verbose
March 05, 2012
Re: Review of Jose Armando Garcia Sancio's std.log
On Mon, 05 Mar 2012 18:30:03 -0500, David Nadlinger <see@klickverbot.at>  
wrote:

> On Monday, 5 March 2012 at 21:55:08 UTC, Steven Schveighoffer wrote:
>> The log aliases use names that are too common.  I think log.info is a  
>> better symbol for logging than just 'info', which could be a symbol in  
>> a myriad of places.  Given that D's symbol lookup rules allow shadowing  
>> of global symbols, this does not work out very well.
>
> Originally, the code used log!info and so on, but it was changed to the  
> current design right after review begin, the rationale being that you  
> could always use »import log = std.log« if you want the extra namespace.

That doesn't help.  Software isn't static.

import std.log;
import other; // defines B

class A : B
{
   void foo()
   {
      info("some info message"); // error! int isn't a function!
   }
}

other.d:

class B
{
   int info; // added later
}

>
>> Like others have stated, I think vlog is a) confusing, and b)  
>> unnecessary.  Even reading the docs, I can't understand what it's used  
>> for, and why it has such different syntax than the normal logging stuff.
>
> I think this been modelled after glog's verbose logging support [1],  
> just like much of the rest of the design (by the way, I think a note  
> about this should added somewhere in the module docs). Does the feature  
> as described in the glog docs make sense to you?

It's good to know the root of where this comes from.  The docs in glog do  
make more sense than the vlog docs.

This may be a doc issue.  I'll have to think about it some more.

-Steve
March 06, 2012
Re: Review of Jose Armando Garcia Sancio's std.log
On Monday, 5 March 2012 at 23:51:29 UTC, Steven Schveighoffer 
wrote:
> On Mon, 05 Mar 2012 18:30:03 -0500, David Nadlinger 
> <see@klickverbot.at> wrote:
>
>> On Monday, 5 March 2012 at 21:55:08 UTC, Steven Schveighoffer 
>> wrote:
>>> The log aliases use names that are too common.  I think 
>>> log.info is a better symbol for logging than just 'info', 
>>> which could be a symbol in a myriad of places.  Given that 
>>> D's symbol lookup rules allow shadowing of global symbols, 
>>> this does not work out very well.
>>
>> Originally, the code used log!info and so on, but it was 
>> changed to the current design right after review begin, the 
>> rationale being that you could always use »import log = 
>> std.log« if you want the extra namespace.
>
> That doesn't help.  Software isn't static.
>
> import std.log;
> import other; // defines B
>
> class A : B
> {
>    void foo()
>    {
>       info("some info message"); // error! int isn't a function!
>    }
> }
>
> other.d:
>
> class B
> {
>    int info; // added later
> }

That is not a counter-argument to something related to this 
library but everything that lies in global namespace.
At its first state both severity levels and the "log" was in 
global namespace. Now only severity levels.

You are also overlooking one crucial fact that this library will 
be part of phobos, standard library. Which requires everyone to 
adopt. When you see codes like this (below), you don't blame 
standard library designers do you?

using namespace std;
int cout;
March 06, 2012
Re: Review of Jose Armando Garcia Sancio's std.log
On Mon, 05 Mar 2012 20:22:05 -0500, so <so@so.so> wrote:

> On Monday, 5 March 2012 at 23:51:29 UTC, Steven Schveighoffer wrote:
>> On Mon, 05 Mar 2012 18:30:03 -0500, David Nadlinger  
>> <see@klickverbot.at> wrote:
>>
>>> On Monday, 5 March 2012 at 21:55:08 UTC, Steven Schveighoffer wrote:
>>>> The log aliases use names that are too common.  I think log.info is a  
>>>> better symbol for logging than just 'info', which could be a symbol  
>>>> in a myriad of places.  Given that D's symbol lookup rules allow  
>>>> shadowing of global symbols, this does not work out very well.
>>>
>>> Originally, the code used log!info and so on, but it was changed to  
>>> the current design right after review begin, the rationale being that  
>>> you could always use »import log = std.log« if you want the extra  
>>> namespace.
>>
>> That doesn't help.  Software isn't static.
>>
>> import std.log;
>> import other; // defines B
>>
>> class A : B
>> {
>>    void foo()
>>    {
>>       info("some info message"); // error! int isn't a function!
>>    }
>> }
>>
>> other.d:
>>
>> class B
>> {
>>    int info; // added later
>> }
>
> That is not a counter-argument to something related to this library but  
> everything that lies in global namespace.
> At its first state both severity levels and the "log" was in global  
> namespace. Now only severity levels.
>
> You are also overlooking one crucial fact that this library will be part  
> of phobos, standard library. Which requires everyone to adopt. When you  
> see codes like this (below), you don't blame standard library designers  
> do you?
>
> using namespace std;
> int cout;

Except 'info', 'error', 'warning' are all common names, likely to be a  
very attractive name for something that has nothing to do with (or cares  
about) logging.  cout is not a common name or even an english word, so  
it's unlikely someone has or wants to create a cout member.

Couple this with the fact that all of these are nouns -- likely candidates  
for fields.

Your argument has some merit, but I would add that my argument is only  
against *common* global namespace names.

Another solution besides using a namespace is to make the names less  
common, like linfo instead of just info.

-Steve
March 06, 2012
Re: Review of Jose Armando Garcia Sancio's std.log
On Tuesday, March 06, 2012 02:22:05 so wrote:
> That is not a counter-argument to something related to this
> library but everything that lies in global namespace.
> At its first state both severity levels and the "log" was in
> global namespace. Now only severity levels.
> 
> You are also overlooking one crucial fact that this library will
> be part of phobos, standard library. Which requires everyone to
> adopt. When you see codes like this (below), you don't blame
> standard library designers do you?
> 
> using namespace std;
> int cout;

Except that cout is not exactly something that would be considered a normal 
variable name. Something like info _is_. This logging module is taking 
incredibly common names and shoving them as far into the global namespace as 
anything can go in D which isn't a compiler built-in. _Not_ a good idea IMHO - 
not without good reason. And I really don't think that this merits it. 
log!info(msg) would work just fine and would be _far_ better.

- Jonathan M Davis
March 06, 2012
Re: Review of Jose Armando Garcia Sancio's std.log
On Tuesday, 6 March 2012 at 01:30:41 UTC, Steven Schveighoffer 
wrote:

> Except 'info', 'error', 'warning' are all common names, likely 
> to be a very attractive name for something that has nothing to 
> do with (or cares about) logging.  cout is not a common name or 
> even an english word, so it's unlikely someone has or wants to 
> create a cout member.
>
> Couple this with the fact that all of these are nouns -- likely 
> candidates for fields.
>
> Your argument has some merit, but I would add that my argument 
> is only against *common* global namespace names.
>
> Another solution besides using a namespace is to make the names 
> less common, like linfo instead of just info.

I have no objections against changing names. For example, instead 
of "info" i use "note" for my logger. Not 100% sure about "error" 
but i think "warning" also implies logging and don't see any use 
case where it would be used as a variable name.
4 5 6 7 8 9 10 11 12
Top | Discussion index | About this forum | D home