I'm fine with Nick's proposal to have two separate votes. Remko On Friday, January 24, 2014, Nick Williams <nicho...@nicholaswilliams.net> wrote:
> By a generic definition, this new Level is certainly an enum. And it's > written in Java. No, it's not a Java-language official enum. But, as far as > byte code goes, it's nearly identical to an official enum. Users who don't > need custom levels will use it EXACTLY like they would use an enum. It's an > improvement over a traditional enum only in that you can extend it. I see > no downsides at all. > > There has obviously been some serious discussion about these topics. We're > not going to come to a total agreement on this. I propose: > > - We have a committers-only vote in this thread on whether to make Level > an extensible enum. > - AFTER having that vote, we have a committers-only vote in the "Levels > added in revision 1560602" thread on whether to add the three levels that > were added. > - We only roll back 1560602 AFTER the second vote is complete and IF the > vote rejects the new levels. > > Nick > > On Jan 23, 2014, at 6:23 AM, Ralph Goers wrote: > > I’m more in favor of this than what you had proposed. To be honest, with > what you proposed I don’t see much value left in keeping the Level enum. > > Yes, I prefer using the enum (obviously, or I wouldn’t have implemented it > that way), but if the consensus is to change it to a class, so be it. > > As for comments on the below, I think it needs a bit of tweaking (I > wouldn’t use Hashtable) but it may be a better option than adding more > levels. Or we could add the extra levels but not add new methods for them > to AbstractLogger. > > Ralph > > On Jan 23, 2014, at 7:49 AM, Paul Benedict <pbened...@apache.org> wrote: > > It's a neat idea but it's not a Java enum. I think one of Ralph's goal was > to allow client code to use enums. I still think we should continue that > path. At any rate, this will hopefully lead to a synthesis of ideas and > agreement. > > > On Thu, Jan 23, 2014 at 8:22 AM, Matt Sicker <boa...@gmail.com> wrote: > > Neat idea. I'd update it for proper concurrency, though. I could write a > mock version of this to show what I mean. > > Matt Sicker > > > On Jan 23, 2014, at 2:42, Nick Williams <nicho...@nicholaswilliams.net> > wrote: > > > > Okay, I finally got a minute to read all of these emails, and... > > > > EVERYBODY FREEZE! > > > > What if I could get you an extensible enum that required no interface > changes and no binary-incompatible changes at all? Sound too good to be > true? I proposed this months ago (LOG4J2-41) and it got shot down multiple > times, but as of now I've heard THREE people say "extensible enum" in this > thread, so here it is, an extensible enum: > > > > public abstract class Level implements Comparable<Level>, Serializable { > > public static final Level OFF; > > public static final Level FATAL; > > public static final Level ERROR; > > public static final Level WARN; > > public static final Level INFO; > > public static final Level DEBUG; > > public static final Level TRACE; > > public static final Level ALL; > > > > > > private static final long serialVersionUID = 0L; > > private static final Hashtable<String, Level> map; > > private static final TreeMap<Integer, Level> values; > > private static final Object constructorLock; > > > > > > static { > > // static variables must be constructed in certain order > > constructorLock = new Object(); > > map = new Hashtable<String, Level>(); > > values = new TreeMap<Integer, Level>(); > > OFF = new Level("OFF", 0) {}; > > FATAL = new Level("FATAL", 100) {}; > > ERROR = new Level("ERROR", 200) {}; > > WARN = new Level("WARN", 300) {}; > > INFO = new Level("INFO", 400) {}; > > DEBUG = new Level("DEBUG", 500) {}; > > TRACE = new Level("TRACE", 600) {}; > > ALL = new Level("ALL", Integer.MAX_VALUE) {}; > > } > > > > > > private static int ordinals; > > > > > > private final String name; > > private final int intLevel; > > private final int ordinal; > > > > > > protected Level(String name, int intLevel) { > > if(name == null || name.length() == 0) > > throw new IllegalArgumentException("Illegal null Level > constant"); > > if(intLevel < 0) > > throw new IllegalArgumentException("Illegal Level int less > than zero."); > > synchronized (Level.constructorLock) { > > if(Level.map.containsKey(name.toUpperCase())) > > throw new IllegalArgumentException("Duplicate Level > constant [" + name + "]."); > > if(Level.values.containsKey(intLevel)) > > throw new IllegalArgumentException("Duplicate Level int > [" + intLevel + "]."); > > this.name = name; > > > >