harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mark Hindess <mark.hind...@googlemail.com>
Subject Re: [general] icu or apr-iconv, which coding library is better?
Date Wed, 16 Apr 2008 09:07:43 GMT

On 16 April 2008 at 11:45, "Alexey Varlamov" <alexey.v.varlamov@gmail.com> 
> 2008/4/16, Nathan Beyer <nbeyer@gmail.com>:
> > I think we need more consensus on this. Is everyone in agreement about
> > dumping log4cxx? Is this patch the appropriate replacement?
> Well, the suggestion has positive responses from me, Xiao-Feng and
> Ilya Berezhniuk, and no specifc opposing arguments. Let's wait last
> 24h, tomorrow I'll commit if no objections.

+1 from me; making the logging more consistent is a big improvement.
Nice work Alexei.

I see no reason not to apply it as is but do have some trivial observations:

0) I'd probably make the change to vm/em/src/EBProfileCollector.cpp:

-    loggingEnabled =  is_info_enabled(LOG_DOMAIN);
-    if (!loggingEnabled) {
-        loggingEnabled = is_info_enabled(catName.c_str());
-    }
+    loggingEnabled =  log_is_info_enabled(LOG_DOMAIN) || 

to be consistent with a couple of similar chunks of code.

1) There is no reason for:

  Property changes on: vm/port/src/logger/logparams.cpp
  Name: svn:executable
     + *

2) It is hard to see the relevance of some changes.  Such as adding the:

  #include "open/hythread.h"

to vm/vmcore/include/stack_trace.h or making native_get_regs_from_jit_context

3) Why were @version and @author tags removed from some files and only @version
tags from others?  Some of these were removed from files that were otherwise
untouched so cynical people might think someone was just trying to make the
number of '-' lines in the patch look "better". ;-)

4) There were lots of spurious changes that could have been made in a
trivial cleanup patch to keep the important changes more transparent.
Things like removing @version tags, renaming pool1 back to pool, and
changes like:

  -#ifndef _VMCORE_LOG_H_
  -#define _VMCORE_LOG_H_
  +#ifndef _VMCORE_LOG_H
  +#define _VMCORE_LOG_H

and removing the '#if 0' block from vm/vmcore/src/class_support/class_impl.cpp.


View raw message