From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21912 invoked by alias); 10 Mar 2008 11:28:16 -0000 Received: (qmail 21905 invoked by uid 22791); 10 Mar 2008 11:28:15 -0000 X-Spam-Status: No, hits=-1.2 required=5.0 tests=AWL,BAYES_00,J_CHICKENPOX_34,J_CHICKENPOX_36,J_CHICKENPOX_44 X-Spam-Check-By: sourceware.org Received: from wildebeest.demon.nl (HELO gnu.wildebeest.org) (83.160.170.119) by sourceware.org (qpsmtpd/0.31) with ESMTP; Mon, 10 Mar 2008 11:27:58 +0000 Received: from dijkstra.wildebeest.org ([192.168.1.29]) by gnu.wildebeest.org with esmtp (Exim 4.63) (envelope-from ) id 1JYgAd-0005wb-BU; Mon, 10 Mar 2008 12:27:55 +0100 Subject: Re: [SCM] master: Refactor Log to Callers. From: Mark Wielaard To: frysk@sourceware.org Cc: Andrew Cagney In-Reply-To: <1204113780.3615.24.camel@dijkstra.wildebeest.org> References: <20080213000530.24916.qmail@sourceware.org> <1202983298.3365.17.camel@dijkstra.wildebeest.org> <1204113780.3615.24.camel@dijkstra.wildebeest.org> Content-Type: text/plain Date: Mon, 10 Mar 2008 11:28:00 -0000 Message-Id: <1205148475.3356.22.camel@dijkstra.wildebeest.org> Mime-Version: 1.0 X-Mailer: Evolution 2.12.3 (2.12.3-3.fc8) Content-Transfer-Encoding: 7bit X-Spam-Score: -4.4 (----) X-IsSubscribed: yes Mailing-List: contact frysk-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Post: List-Help: , Sender: frysk-owner@sourceware.org X-SW-Source: 2008-q1/txt/msg00141.txt.bz2 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 > > > > > > * LinuxCoreProc.java: Use Log.CALLER. > > > * LinuxCoreTask.java: Ditto. > > > > > > frysk-sys/frysk/rsl/ChangeLog > > > 2008-02-12 Andrew Cagney > > > > > > * 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