Gary, The change only affects Appender, Layout is still generic. Appender does not need to be generic (it can just return a Layout<? extends Serializable> from its getLayout() method).
This change does not introduce any type casts (afaics). Remko On Wed, Aug 14, 2013 at 10:11 PM, Gary Gregory <[email protected]>wrote: > 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 >
