Re: [SCM] master: Refactor Log to Callers.

[Mark Wielaard <mark@klomp.org>]

Hi Andrew,

On Wed, 2008-02-27 at 13:03 +0100, Mark Wielaard wrote:
> On Thu, 2008-02-14 at 11:01 +0100, Mark Wielaard wrote:
> > On Wed, 2008-02-13 at 00:05 +0000, cagney@sourceware.org wrote:
> > >     Refactor Log to Callers.
> > >     
> > >     frysk-core/frysk/proc/dead/ChangeLog
> > >     2008-02-12  Andrew Cagney  <cagney@redhat.com>
> > >     
> > >     	* LinuxCoreProc.java: Use Log.CALLER.
> > >     	* LinuxCoreTask.java: Ditto.
> > >     
> > >     frysk-sys/frysk/rsl/ChangeLog
> > >     2008-02-12  Andrew Cagney  <cagney@redhat.com>
> > >     
> > >     	* Node.java (set(Level)): Return the node.
> > >     	* Callers.java: Extract from Log.java.
> > >     	* TestCallers.java: New.
> > >     	* Log.java (CALLER): New.
> > >     	(CALLERS): New.
> > >     	(dump(Object)): Check for null.
> > >     	(set(PrintWriter)): New.
> > >     	(set(PrintStream)): Return old writer.
> > 
> > If you refactor working code could you please explain why.
> > The functionality does seem almost the same now, but it is far more
> > complex to see why it works. Depending on Callers.toString() in
> > Log.dump() seems somewhat fragile unless you fix the call depth that
> > method is called from somehow. There should at least be a warning in
> > Log.java to not nest log() and dump() calls that might take a CALLER or
> > CALLERS. Also the original code had comments for each method and field
> > explaining why it was necessary, what it did and how to use it. It would
> > be nice if the new code had the same (why is the method
> > Callers.callers() included for example?)
> 
> I see you refactored the logging code some more, but didn't add any
> documentation about the above. It would be good to have some
> documentation discussing the above in the code so others can help
> maintain it later on.

Ping. Please do add some documentation here. It really helps.

Thanks,

Mark