I still have a major bug to fix in cal10n. Next, is a review of Carl's
contributions in ch.qos.logback.*.net package. Once that is done, I'll
look at new bug reports and pull requests.

Thus, unfortunately I have not yet had a chance to look at your
changes to AppenderTrackerImpl and co. In my defense, my todo list is
ever expanding. Even when I spend the whole day working on logback, at
the end of the day, there are more logback-related items on my todo
list than the day started with. Not to rant but bug reports and pull
requests are generated at a rate faster than I can cope with them.

As jira.qos.ch states, popular issues, those with most votes and
highest activity, get treated first.


On 13.04.2013 10:24, David Roussel wrote:
Ceki,

Would you be interested in accepting this change? Is there anything else you 
feel needs to be done?

I'd be interested in seeing it land in the next version.

David

On 13 Apr 2013, at 01:01, "Becker, Thomas" <[email protected]> wrote:

There was already a Map there.  To your point though, that bit of rework to 
AppenderTrackerImpl was not strictly necessary but I felt the existing code, 
which essentially re-implemented LinkedHashMap in a clumsy way was  pretty low 
hanging fruit.  I'd be willing to make additional changes if the maintainer 
showed interest in accepting the patch, but my immediate need has passed.  My 
observation has been that the majority of the pull requests to Logback seem to 
get no comments at all.  But I went ahead and reopened this one here: 
https://github.com/qos-ch/logback/pull/107

-Tommy

________________________________________
From: Logback-user [[email protected]] on behalf of diroussel 
[[email protected]]
Sent: Friday, April 12, 2013 12:12 PM
To: [email protected]
Subject: Re: [logback-user] SiftingAppender and RollingFileAppender,    roll on 
close.

Thomas,

That functionality looks good.

In order to get accepted, maybe you can make your changes to
AppenderTrackerImpl less invasive?

In AppenderTrackerImpl, you are subclassing LinkedHashMap instead of using
ch.qos.logback.classic.pattern.LRUCache.  But really there is no need for a
map. Just keep the existing head and tail references, keep a count
variablle. If the count gets to high, then remove entries from the tail.

I like that you've added unit tests and the code formatting is consistent
with the rest of the code base.

David




--
Ceki
65% of statistics are made up on the spot
_______________________________________________
Logback-user mailing list
[email protected]
http://mailman.qos.ch/mailman/listinfo/logback-user

Reply via email to