logging-log4j-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Nick Williams <nicho...@nicholaswilliams.net>
Subject Re: svn commit: r1514021 [2/2] - in /logging/log4j/log4j2/trunk: core/src/main/java/org/apache/logging/log4j/core/appender/ core/src/main/java/org/apache/logging/log4j/core/appender/rewrite/ core/src/main/java/org/apache/logging/log4j/core/async/ core/src/...
Date Sat, 17 Aug 2013 14:25:33 GMT
I will support the commit. I have to admit, I do like the strictness of it. It allows <appender-ref>
while disallowing <a-p-p-e-n-d-e-r--r-e-f>, which is as close to strict schema compliance
that I think I'm going to get anyone to agree to. :-)

But we must document the heck out of it and we must be vigilant to ALWAYS create an alias
for every plugin AND every attribute that has multiple words. Otherwise we're creating more
room for users to trip up.

Nick

On Aug 17, 2013, at 9:18 AM, Ralph Goers wrote:

> So I think Remko and I agree that consistency is good but that there are two cases where
exceptions should be made.  If I commit the change to alias those two things is anyone so
strongly against it that they would veto it?  If not then I would like to commit it.
> 
> Ralph
> 
> On Aug 17, 2013, at 12:45 AM, Remko Popma <remko.popma@gmail.com> wrote:
> 
>> ApPeNdeRrEf  may look visually as confusing as a-p-p-e-n-d-e-r-r-e-f r-e-f,
>> but that is not the point.
>> 
>> I think the rule "config files are case-insensite" is much a more intuitive rule
than "all hyphens are stripped from attribute and element names".
>> 
>> To me, the "-ref" extension in the <appender-ref> element is a nice, visually
distinct indicator that this element points to another element in the configuration. To me,
these pointer elements/attributes are special elements and attributes and we actually *lose*
something if we bend over backwards to make them look consistent with other elements. It's
okay if they look different because they *are* different.
>> 
>> If we prefer not to have aliases then I propose we revert back the appender-ref and
error-ref elements to the hyphen version. I think they are better names. 
>> 
>> 
>> 
>> On Sat, Aug 17, 2013 at 3:48 PM, Gary Gregory <garydgregory@gmail.com> wrote:
>> Dancing angels and pin heads: 
>> 
>> Yep, the patch I provided allows for <a-p-p-e-n-d-e-r-r-e-f r-e-f="Console"/>
but the point is that is allows <appender-ref ref="Console>
>> 
>> But! the current code also allows <ApPeNdeRrEf ref="Console/>
>> 
>> How is that any better/worse, more/less confusing?
>> 
>> BTW, are attributes also case-insensitive? <ApPeNdeRrEf ReF="Console/>
>> 
>> Gary
>> 
>> 
>> 
>> On Sat, Aug 17, 2013 at 2:38 AM, Ralph Goers <ralph.goers@dslextreme.com> wrote:
>> OK.  In general I guess I agree with your philosophy.  However, I consider stripping/ignoring
hyphens bad because then <a-p-p-e-n-d-e-r-r-e-f r-e-f="Console"/> becomes valid.  The
ONLY reason I wanted aliases is because I really believe users who are used to the "other"
logging frameworks are constantly going to screw up and do <appender-ref ref="abc"/>
instead of <AppenderRef ref="abc"/> simply because they are used to it.  However, if
my choice is between stripping or leaving it the way it is then I vote to leave it the way
it is.  Again, I just detest the idea of stripping hyphens.
>> 
>> Ralph
>> 
>> On Aug 16, 2013, at 11:28 PM, Nick Williams wrote:
>> 
>>> Alright. Time to chime in I suppose, since I'm being quoted now. :-)
>>> 
>>> I like consistency. I like the same rules to apply to all parts of the configuration.
For example:
>>> 
>>> - If we decide that an element should be PascalCase, then they should ALL be
PascalCase.
>>> - If we decide that an attribute should be camelCase, then they should ALL be
camelCase.
>>> - If we decide that an element and/or attribute should be case-insensitive, then
ALL elements AND attributes should be case-insensitive.
>>> - If we decide that an element and/or attribute should allow hyphens between
words, then ALL elements AND attributes should allow hyphens between words.
>>> 
>>> This last one is a key point here. Providing aliases would not be a sane way
to do this, because what if a developer added an attribute but forgot to create a hyphenated
alias? Suddenly, all elements and attributes would allow hyphenation—except that one attribute.
This is a consistency failure.
>>> 
>>> I'm not arguing against aliases, necessarily. I just think they're a Bad way
to solve the hyphenation dispute (yes, capital Bad). With the hyphenation issue solved otherwise
(either by disallowing hyphens or by stripping hyphens automatically), I no longer see a compelling
need for aliases. The addition of aliases also makes the task for users of extending Log4j
/ writing plugins for Log4j more confusing.
>>> 
>>> If I had to chose what we were going to do here, these are my preferences/priorities,
in order from most important to least important:
>>> 
>>> - Consistency, consistency, consistency.
>>> - A strict schema that must be validated against for the Log4j configuration
to work. No case insensitivity, no stripping of hyphens.
>>> - All lowercase, hyphenated elements AND attributes. No PascalCase, no camelCase.
>>> - camelCase elements AND camelCase attributes.
>>> - PascalCase elements AND PascalCase attributes.
>>> 
>>> Nick
>>> 
>>> On Aug 17, 2013, at 1:14 AM, Ralph Goers wrote:
>>> 
>>>> On Aug 10 Nick said "I actually really like hyphenated attributes, but I
like consistency better.".  However, that doesn't imply that he is going to like allowing
'-' to appear anywhere and be stripped out.  Providing aliases would be a more sane way to
do that.
>>>> 
>>>> Ralph
>>>> 
>>>> 
>>>> 
>>>> On Aug 16, 2013, at 10:57 PM, Gary Gregory wrote:
>>>> 
>>>>> On Sat, Aug 17, 2013 at 1:44 AM, Ralph Goers <ralph.goers@dslextreme.com>
wrote:
>>>>> So far yours is the only vote for that.  Anyone else?
>>>>> 
>>>>> Whomever else mentioned it in the first place! ;) I can't recall who...
but it's 2am here...
>>>>> G
>>>>> 
>>>>> Ralph
>>>>> 
>>>>> On Aug 16, 2013, at 10:19 PM, Gary Gregory wrote:
>>>>> 
>>>>>> I think we should do both. 
>>>>>> 
>>>>>> Gary
>>>>>> 
>>>>>> On Aug 16, 2013, at 21:59, Ralph Goers <ralph.goers@dslextreme.com>
wrote:
>>>>>> 
>>>>>>> Easily done, assuming we have consensus.   I am hearing two options:
>>>>>>> 1) strip '-' characters from element names.
>>>>>>> 2) allow aliases for element names.
>>>>>>> 
>>>>>>> These are not mutually exclusive.  I see no reason not to go
ahead with number 2 and we can continue to discuss where else number 1 might be used.
>>>>>>> 
>>>>>>> Ralph
>>>>>>> 
>>>>>>> On Aug 16, 2013, at 2:21 PM, Remko Popma wrote:
>>>>>>> 
>>>>>>>> Ralph,
>>>>>>>> Don't forget the error-ref attribute for AsyncAppender. 
>>>>>>>> 
>>>>>>>> Remko
>>>>>>>> 
>>>>>>>> On Saturday, August 17, 2013, Ralph Goers wrote:
>>>>>>>> I'm not in favor of just allowing arbitrary '-' characters
wherever users want. But allowing aliases makes it possible to allow for variations.  I already
have this working.
>>>>>>>> 
>>>>>>>> Ralph
>>>>>>>> 
>>>>>>>> 
>>>>>>>> On Aug 16, 2013, at 11:39 AM, Gary Gregory wrote:
>>>>>>>> 
>>>>>>>>> On Fri, Aug 16, 2013 at 2:38 PM, Gary Gregory <garydgregory@gmail.com>
wrote:
>>>>>>>>> On Fri, Aug 16, 2013 at 2:28 PM, Scott Deboy <scott.deboy@gmail.com>
wrote:
>>>>>>>>> I'm not sure if this ship has fully sailed, but I'd prefer
to see us
>>>>>>>>> stick with he dash format due to folks being familiar
with it from
>>>>>>>>> log4j 1.
>>>>>>>>> 
>>>>>>>>> That's a thin argument IMO considering you'll have to
read the version 2 config docs to get off the ground anyway, even if you know your way around
version 1. 
>>>>>>>>> 
>>>>>>>>> And this is also an opportunity to make our config code
even fancier by normalizing '-' chars ;)
>>>>>>>>> 
>>>>>>>>> Gary
>>>>>>>>>  
>>>>>>>>> 
>>>>>>>>> Gary
>>>>>>>>>  
>>>>>>>>> 
>>>>>>>>> Scott
>>>>>>>>> 
>>>>>>>>> On 8/16/13, Gary Gregory <garydgregory@gmail.com>
wrote:
>>>>>>>>> > On Fri, Aug 16, 2013 at 1:13 PM, Ralph Goers
>>>>>>>>> > <ralph.goers@dslextreme.com>wrote:
>>>>>>>>> >
>>>>>>>>> >> I'm adding an aliases attribute to the Plugin
annotation.
>>>>>>>>> >>
>>>>>>>>> >
>>>>>>>>> > Hold on to your horses ;)
>>>>>>>>> >
>>>>>>>>> > Another way to look at this is that our config parsing
that is already
>>>>>>>>> > case-insensitive could be augmented to strip out
"-"s, no aliases needed.
>>>>>>>>> >
>>>>>>>>> > As someone pointed out here, some folks like-to-talk-like-this
(see JPA).
>>>>>>>>> >
>>>>>>>>> > Gary
>>>>>>>>> >
>>>>>>>>> >
>>>>>>>>> >>
>>>>>>>>> >> On Aug 16, 2013, at 6:38 AM, Remko Popma wrote:
>>>>>>>>> >>
>>>>>>>>> >> Maybe we just need another plugin for the 2nd
name then. Subclass or
>>>>>>>>> >> delegate?
>>>>>>>>> >>
>>>>>>>>> >> On Friday, August 16, 2013, Ralph Goers wrote:
>>>>>>>>> >>
>>>>>>>>> >>> I had the same thought. People switching
from log4j 1 or logback will
>>>>>>>>> >>> probably make that mistake a lot. Plus this
breaks virtually everyone
>>>>>>>>> >>> currently using Log4j 2.  The problem is
that I don't think there is
>>>>>>>>> >>> currently a way for a plugin to have 2 names.
>>>>>>>>> >>>
>>>>>>>>> >>> Sent from my iPad
>>>>>>>>> >>>
>>>>>>>>> >>> On Aug 16, 2013, at 6:03 AM, Remko Popma
<remko.popma@gmail.com> wrote:
>>>>>>>>> >>>
>>>>>>>>> >>> Would it be an idea to support both appender-ref
and appenderRef
>>>>>>>>> >>> attributes?
>>>>>>>>> >>>
>>>>>>>>> >>>
>>>>>>>>> >>>
>>>>>>>>> >>> On Thu, Aug 15, 2013 at 10:22 AM, Gary Gregory
>>>>>>>>> >>> <garydgregory@gmail.com>wrote:
>>>>>>>>> >>>
>>>>>>>>> >>> I never thought that Log4J 2 configuration
files should be backward
>>>>>>>>> >>> compatible with version 1, and even less
so with a different product.
>>>>>>>>> >>>
>>>>>>>>> >>> Gary
>>>>>>>>> >>>
>>>>>>>>> >>>
>>>>>>>>> >>> On Wed, Aug 14, 2013 at 8:51 PM, Ralph Goers
>>>>>>>>> >>> <ralph.goers@dslextreme.com>wrote:
>>>>>>>>> >>>
>>>>>>>>> >>> Now that I see this it kind of scares me.
 Log4j 1.x and Logback both
>>>>>>>>> >>> use
>>>>>>>>> >>> appender-ref. Anyone using Log4j 2 will
now be broken.
>>>>>>>>> >>>
>>>>>>>>> >>> On Aug 14, 2013, at 1:05 PM, ggregory@apache.org
wrote:
>>>>>>>>> >>>
>>>>>>>>> >>> > Modified:
>>>>>>>>> >>> logging/log4j/log4j2/trunk/core/src/test/resources/log4j2-config.xml
>>>>>>>>> >>> > URL:
>>>>>>>>> >>>
>>>>>>> 
>>>>> 
>>>>> 
>>>>> 
>>>>> 
>>>>> -- 
>>>>> E-Mail: garydgregory@gmail.com | ggregory@apache.org 
>>>>> Java Persistence with Hibernate, Second Edition
>>>>> JUnit in Action, Second Edition
>>>>> Spring Batch in Action
>>>>> Blog: http://garygregory.wordpress.com 
>>>>> Home: http://garygregory.com/
>>>>> Tweet! http://twitter.com/GaryGregory
>>>> 
>>> 
>> 
>> 
>> 
>> 
>> -- 
>> E-Mail: garydgregory@gmail.com | ggregory@apache.org 
>> Java Persistence with Hibernate, Second Edition
>> JUnit in Action, Second Edition
>> Spring Batch in Action
>> Blog: http://garygregory.wordpress.com 
>> Home: http://garygregory.com/
>> Tweet! http://twitter.com/GaryGregory
>> 


Mime
View raw message