Feedback requested re submitted patches

[Kitching Simon <Si...@orange.ch>]

Hi all,

Early last week I sent in a few submissions.
Is there any chance I could get some feedback, ie
whether they are likely to be accepted or not, and
if not what the problems might be?

The submissions were:

(a)
PATCH: Priority.java : add method Priority toPriority(int, Priority)
[tuesday 6 feb]
I did get an initial reply from Ceki, but no response to my reply.

I can see valid reasons for refusing this patch; see Ceki's email re this
method
only being intended for use during deserialisation of Priority objects.

An alternative that seems satisfactory to me would be to update the comments

on the Priority class toPriority(int) method to indicate that (1) it is not
intended 
to be called directly by users, and (2) that it is only for use in
deserialisation and 
therefore does not need to be over-ridden by custom priority classes.

While on this subject, if this method *is* used for deserialisation somehow,
why does it take an int, not an int and a string? I am concerned about how
serialisation/deserialisation of custom priorities will work....

(b)
PATCH: New class: PriorityRangeFilter [tuesday 6 feb]

I could understand if this is rejected on the grounds that it is not going
to be
very heavily used - still, it could be useful from time to time.

(c)
PATCH: Allow custom priority classes in <param> tags and Property
Configurator

I think this is a very useful patch. I really would like to see this go into
log4j, as
without it custom priorities are fairly crippled. The concept of specifying
custom
priorities as "classname#level" seems to me to be consistent with other
parts
of log4j.

To make use of this patch, various other classes would have to be
(trivially)
modified to use the new OptionConverter.toPriority method. I am willing to
do this, or someone else can take the 10 minutes necessary to do the
changes (eg to PropertyConfigurator). Of course, testing will take more
time..

There is one possible issue: it depends upon dynamic method invocation,
which will
make life difficult for the log4cpp project (port of log4j to c++). An
alternative would
be to completely rethink the current approach to custom priority classes,
but
that's a big task! Personally, I think that this potential problem is not
enough to
stop this patch .. though of course that's up to you guys to decide.

(d)
PATCH: New classes: DatagramStreamAppender & friends [wed 7 feb]

I think this is a very useful addition to log4j.

[[nb: email title is wrong: should be DatagramStringAppender & friends]]


Regards,

Simon

Re: Feedback requested re submitted patches

[Ceki Gülcü <cg...@qos.ch>]

Hi Simon,

At 13:55 12.02.2001 +0100, you wrote:
>Hi all,
>
>Early last week I sent in a few submissions.
>Is there any chance I could get some feedback, ie
>whether they are likely to be accepted or not, and
>if not what the problems might be?
>
>The submissions were:
>
>(a)
>PATCH: Priority.java : add method Priority toPriority(int, Priority)
>[tuesday 6 feb]
>I did get an initial reply from Ceki, but no response to my reply.

It has been integrated. See the latest code in our CVS rep. 

>(b)
>PATCH: New class: PriorityRangeFilter [tuesday 6 feb]
>
>I could understand if this is rejected on the grounds that it is not going
>to be
>very heavily used - still, it could be useful from time to time.

OK. I'll look at it once again.

>(c)
>PATCH: Allow custom priority classes in <param> tags and Property
>Configurator

It is already possible to specify a custom priority in DOMConfigurator. Regardless, I think it is a good patch. We should add it. 

>(d)
>PATCH: New classes: DatagramStreamAppender & friends [wed 7 feb]
>
>I think this is a very useful addition to log4j.

It's in my to do list colored in red. Thanks for sending a reminder. :-) Ceki


----
Ceki Gülcü           e-mail: cgu@qos.ch (preferred)
av. de Rumine 5              ceki_gulcu@yahoo.com
CH-1005 Lausanne          
Switzerland            Tel: ++41 21 351 23 15