"Proper documentation" is the key phrase. :-)
On Thu, Jul 18, 2013 at 7:25 PM, Nick Williams < [email protected]> wrote: > Okay. Hold everything! Lol... > > I started working on this change and then realized something. "Suppress" > means "to forcibly put an end to," "restrain," or "prevent the development, > action, or expression of." However, "ignore" means "refuse to take notice > of or acknowledge; disregard intentionally" or "fail to consider (something > significant)." > > Though not guaranteed, I can see how a user would mistake the word > "ignore" to mean that exceptions aren't even logged, while "suppress" would > be more obvious to mean they aren't thrown (but also confusing with Java > suppressed exceptions as others pointed out). I don't think it's a big deal > because no matter what we do we're going to have to properly document this > field. But I just wanted to make sure everyone had thought about this > before we proceeded. > > Nick > > On Jul 18, 2013, at 6:58 PM, Nick Williams wrote: > > > Okay. I'll proceed with the change. > > > > Nick > > > > On Jul 18, 2013, at 2:08 PM, Ralph Goers wrote: > > > >> First, I appreciate having the discussion before a code change like > this is made. If that had been done I probably would have vetoed it. But > this is the ASF where everybody gets a single voice and a single vote. > Although I dislike this change and the effect it will have on my users I > won't veto it if that is what the majority wants. > >> > >> Ralph > >> > >> On Jul 18, 2013, at 11:06 AM, Nick Williams wrote: > >> > >>> Hmmm. So it sounds like we're at an impasse. Everyone except Ralph > seems to agree to renaming it ignoreExceptions, bug Ralph said the below in > opposition. Ralph, can you clarify a little? Are you objecting to just > renaming the XML/JSON attribute (which, really, is the only thing that > would break your teams' configurations)? Or are you objecting to > ignoreExceptions completely? I can't tell whether you're okay with the > isExceptionSuppressed method being renamed. > >>> > >>> I would just throw it out there that, while I understand that it would > be inconvenient to have to modify all your configurations, this IS beta > software. It is not GA yet (hopefully next month or so). Anyone who uses > beta software does so with the understanding that there could be breaking > changes before GA. If you didn't want to take that risk, I respectively > submit that you shouldn't have used beta software in production. I'm not > saying, "You're wrong." I'm just questioning the motives behind your > opposition. In short: > >>> > >>> If we all agree that ignoreExceptions is a better name that > suppressExceptions (inconvenience aside), now is the time to change it. We > can't after GA. > >>> > >>> Now I'm not sure how to proceed. :-P > >>> > >>> Nick > >>> > >>> On Jul 18, 2013, at 1:02 AM, Ralph Goers wrote: > >>> > >>>> I do not have a problem with renaming handleExceptions to > exceptionSuppressed. I do have a problem with renaming supressExceptions > to ignoreExceptions, primarily because I have a bunch of teams using Log4j > 2 in production and they would have to modify their configurations when > they upgrade. Furthermore, I can't in my wildest dreams imagine anyone > getting confused over this parameter and suppressed Throwables. > >>>> > >>>> FWIW - handleExceptions means that the Appender "handles" the > exception (i.e. it is suppressed). I don't recall why the variables don't > match - I think I might have originally exposed "handleExceptions" and > found that to be ambiguous and renamed the config param but not the > internal variable. > >>>> > >>>> Ralph > >>>> > >>>> On Jul 17, 2013, at 2:42 PM, Nick Williams wrote: > >>>> > >>>>> Appender specifies a method, isExceptionSuppressed(), which > indicates whether exceptions thrown while appending events should be > suppressed (logged instead of re-thrown). > >>>>> > >>>>> AbstractAppender implements this method with a private > handleExceptions field and a handleExceptions constructor argument. > isExceptionSuppressed() returns handleExceptions (so, supposedly, "handle > exceptions" means "take care of exceptions instead of the user having to > take care of exceptions"). > >>>>> > >>>>> Everybody that extends AbstractAppender uses the same > handleExceptions constructor argument. They all define a suppressExceptions > XML attribute that is assigned to the handleExceptions constructor argument > in the static plugin factory method. > >>>>> > >>>>> This is all very confusing to me. I just realize that I have > misunderstood "handleExceptions" this whole time in the database appenders > and have assumed it was the opposite of isExceptionSuppressed() / > suppressExceptions (and, thus, have written incorrect code). > >>>>> > >>>>> Does anyone have a problem with me renaming handleExceptions to > exceptionSuppressed (to match the JavaBean isExceptionSuppressed method) to > make this less confusing? > >>>>> > >>>>> Nick > >>>>> --------------------------------------------------------------------- > >>>>> To unsubscribe, e-mail: [email protected] > >>>>> For additional commands, e-mail: [email protected] > >>>>> > >>>> > >>>> > >>>> --------------------------------------------------------------------- > >>>> To unsubscribe, e-mail: [email protected] > >>>> For additional commands, e-mail: [email protected] > >>>> > >>> > >>> > >>> --------------------------------------------------------------------- > >>> To unsubscribe, e-mail: [email protected] > >>> For additional commands, e-mail: [email protected] > >>> > >> > >> > >> --------------------------------------------------------------------- > >> To unsubscribe, e-mail: [email protected] > >> For additional commands, e-mail: [email protected] > >> > > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > > -- Cheers, Paul
