On Wed, Aug 14, 2013 at 10:45 AM, Remko Popma <[email protected]> wrote:
> Does toSerializable need to be a single method? > Can't we have separate methods that return different things? > One method that returns a String, and a different method that returns a > LogEvent copy? > See Ralph's message on this thread dated Sun, Aug 11, 2013 at 3:47 PM. I think that addresses the issue. I am guessing the 3:47 PM is in my timezone, UTC-05. Gary That would simplify things a lot, I think. > > > On Wed, Aug 14, 2013 at 11:37 PM, Gary Gregory <[email protected]>wrote: > >> On Wed, Aug 14, 2013 at 9:52 AM, Remko Popma <[email protected]>wrote: >> >>> 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). >>> >> >> This might be academic, but it would have to introduce type casts since >> we have two kinds of layouts: String and LogEvent. >> >> For example: >> >> LogEvent logEvent = null; >> logEvent = new SerializedLayout().toSerializable(logEvent); >> String xmlFragment = XMLLayout.createLayout(null, null, null, >> null, null, null).toSerializable(logEvent); >> >> Gary >> >> >> >>> 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 >>>> >>> >>> >> >> >> -- >> 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 >> > > -- 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
