[logback-dev] pulling in http://github.com/nurkiewicz/logback

Ceki Gülcü ceki at qos.ch
Wed Mar 31 17:32:43 CEST 2010


Hello Thomasz,

While pulling in from your repo, I noticed a few things.

- In CustomDBNameResolver you now check for the null names of enum names. For 
example,
   if (tableNameOverrides.get(tableName) != null)
became
  if (tableNameOverrides.get(tableName.name()) != null)

Since the tableName variable is an enum, its name cannot be null if tableName is 
not null.

However, in the same method changing the return value from
    return tableNameOverrides.get(tableName);
to
    return tableNameOverrides.get(tableName.name());
although equivalent, is clearer.

Given that CustomDBNameResolver cannot be configured via a config file
on account of Joran's lack of support for two-argument setters, I
think it should be either removed or changed so that it takes the
modified column names via a map of type Map<Enum, String>. If you
prefer the latter option, then please enter a jira issue requesting to
add support for maps in joran. In the mean time I am going to remove
CustomDBNameResolver because currently it's just noise.

- In FileAppenderResilienceTest for some reason you removed the
@Ignore annotation on a test called manual() which never exits. As the
name suggests, it's intended for manual testing.

- In IncludeActionTest you have added an import statement for
org.junit.Ignore but do not use it.

The test cases you provide are quite nice. How come you did not use
the hamcrest type assertions built into Junit 4.4+ and chose
fest-assert instead?

Many thanks for your contributions,

--
Ceki


More information about the logback-dev mailing list