On Wed, Aug 14, 2013 at 4:32 AM, Remko Popma <[email protected]> wrote:
> Patch looks nice. Much fewer "raw type" compiler warnings. Seems like a > big improvement to me. > > A small improvement on the patch would be to remove the (now unnecessary) > @param <T> javadoc comments. I have that additional change done in my local > workspace. > > I wouldn't mind committing this but I don't want to disrupt anyone's > work-in-progress: the patch modifies about 125 files. > > Is everyone ok with me committing this? I'll hold off for a day or two (or > less if we're all ok with this). > Hi All: Please also consider these thought paths: - The raw compiler warnings can also be considered as a 'task not finished' warning. That is, I did not go all the way in pushing generics through the whole code base. - Consider the Appender/Layout generics separately from the other generics in the code. I think this is already the case based on other messages I've read, but I wanted to make sure 'the baby does not get thrown out with the bath water' (a goofy saying here in the US). - Weigh was is 'correct' vs. what is 'convenient' vs. compiler warnings. - What is the goal? For me, the goal of the generics here are multiple: -- Have the ability to write code that, for example, can configure and start Log4j and then manipulate it without type casts. -- Avoid type casts in general. -- Cause compile time errors instead of a runtime errors when extending Log4j. Gary > > Remko > > > On Monday, August 12, 2013, Ralph Goers wrote: > >> I've briefly glanced at your patch and it looks reasonable to me. It >> leaves the Layout to continue to use generics but removes the generics from >> the Appenders. That seems like a reasonable thing to do. >> >> Ralph >> >> On Aug 11, 2013, at 6:34 PM, Henning Schmiedehausen wrote: >> >> Ok, that makes a lot of sense. I have reworked the patch to leave these >> alone and put it on a different git branch. I also opened a ticket: >> https://issues.apache.org/jira/browse/LOG4J2-343 and attached it as a >> patch so that if you do not want to deal with git, you can get it from >> there. >> >> I added instructions on how to get the patch from git to the ticket. >> >> >> On Sat, Aug 10, 2013 at 4:40 PM, Nick Williams < >> [email protected]> wrote: >> >>> >>> On Aug 10, 2013, at 6:31 PM, Henning Schmiedehausen wrote: >>> >>> Hi, >>> >>> I was toying with the log4j 2 API for a new project and I stumbled over >>> the fact that it uses a generic for Appender<T> without actually being >>> generic. The only generic part is the Layout. So as a result there is this >>> weird construct of Appender<SomeSerializableType> which is actually >>> dictated by the layout in use. >>> >>> >>> I'm relatively new to the team, so I don't know much about the reasoning >>> behind making Appender generic, so I can't speak to that. I'm not >>> personally opposed to removing these generics, but that is a HUGE change. >>> >>> This leads to really interesting constructs such as >>> >>> public abstract class AbstractDatabaseAppender<T extends >>> AbstractDatabaseManager> extends AbstractAppender<LogEvent> >>> >>> >>> Well this is a very different case. The <LogEvent> here is about Layout, >>> just as you said. The <T extends AbstractDatabaseManager> is completely >>> unrelated to Layout and I am _not_ in favor of removing these generics. >>> >>> I was wondering whether this is necessary as it makes the API very >>> cumbersome to use and read so I removed the generic from Appender and >>> subsequently went through the log4j 2 code base and mostly removed stuff >>> that was no longer needed once that was gone. The result is at >>> >>> https://github.com/apache/logging-log4j2/pull/1 >>> >>> >>> The Apache GitHub repository is just a mirror of our SVN repository. We >>> can't accept or use any pull requests there. You need to generate an SVN >>> patch and attach it to whatever JIRA you create. (As such, you should close >>> this pull request.) >>> >>> I will also file a JIRA for this. >>> >>> I know that the 2.0 release should be coming soon (being at beta8), but >>> I feel that making that change in the API before it is set in stone with >>> 2.0 woulc be really beneficial for anyone who wants to port code to 2.0 / >>> write new code. >>> >>> >>> I'm sure there will be plenty of discussion about this over the next few >>> days. >>> >>> Thanks for considering, >>> Henning >>> >>> >>> Nick >>> >> >> >> -- E-Mail: [email protected] | [email protected] Java Persistence with Hibernate, Second Edition<http://www.manning.com/bauer3/> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/> Spring Batch in Action <http://www.manning.com/templier/> Blog: http://garygregory.wordpress.com Home: http://garygregory.com/ Tweet! http://twitter.com/GaryGregory
