4:30 a.m....

And I knew you weren't asking for reviews, but I would like to review it if 
that's okay.

N

On Aug 14, 2013, at 4:18 AM, Remko Popma wrote:

> I didn't mean to ask for more reviews for the patch (although always welcome 
> of course), I just didn't want to modify many files without warning. Are you 
> in the middle of a big work-in-progress?
> 
> (And isn't it like 3 AM where you are? :-)
> 
> Remko
> 
> On Wednesday, August 14, 2013, Nick Williams wrote:
> Please hold off until the weekend if you can. I won't get a chance to look 
> over it until then.
> 
> Nick
> 
> On Aug 14, 2013, at 3:32 AM, Remko Popma 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).
>> 
>> 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
>>> 
>> 
> 

Reply via email to