Done, tracked through https://issues.apache.org/jira/browse/LOG4J2-1286
Gary On Fri, Feb 19, 2016 at 12:29 PM, Ralph Goers <[email protected]> wrote: > Yes, I am OK with this. > > Ralph > > On Feb 19, 2016, at 1:28 PM, Gary Gregory <[email protected]> wrote: > > If we all agree that MS should be deprecated then yes, let's have one task > to remove new 2.6 MS APIs and deprecate existing 2.5 ones. > > Ralph? Are you OK with this? > > Then we can continue refining. > > Gary > On Feb 18, 2016 8:23 AM, "Remko Popma" <[email protected]> wrote: > >> I addressed the problem in >> https://issues.apache.org/jira/browse/LOG4J2-1280 >> LambdaUtil now correctly handles the case where Supplier<?>s returns a >> Message object. >> I believe that MessageSupplier can now be deprecated. >> >> It may also make sense to remove the new traceEntry/traceExit methods >> introduced in LOG4J2-1255 that use MessageSupplier: their Supplier<?> >> equivalent should be sufficient. >> >> On Tue, Feb 16, 2016 at 8:34 AM, Matt Sicker <[email protected]> wrote: >> >>> The wildcard bounds are checked at compile time though I thought. Plus, >>> despite the type erasure, there's still a bit of info available through >>> reflection, so it's not entirely lost. >>> >>> On 15 February 2016 at 16:23, Remko Popma <[email protected]> wrote: >>> >>>> The wildcard bounds are no help since they get erased, so we cannot >>>> have separate methods for Supplier<? extends Message> and Supplier<?>. >>>> (This is why I originally introduced MessageSupplier.) >>>> >>>> I don't mind deprecating MessageSupplier, but the point is that in the >>>> implementation of these logging methods we need to be very careful: >>>> >>>> Generally, if log(Supplier<?>) supplies an object of type Message it >>>> doesn't need to be wrapped in an ObjectMessage, where if any other Object >>>> is supplied it _does_ need to be wrapped. >>>> >>>> Similarly for where the Supplier supplies param values: if the supplied >>>> value is a Message you need to _unwrap_ it by calling getFormattedString() >>>> on it. The current impl of traceEntry/Exit does not do this correctly, I >>>> reported this as a bug in LOG4J2-1255. >>>> >>>> Sent from my iPhone >>>> >>>> On 2016/02/16, at 2:21, Gary Gregory <[email protected]> wrote: >>>> >>>> On Mon, Feb 15, 2016 at 9:17 AM, Gary Gregory <[email protected]> >>>> wrote: >>>> >>>>> On Mon, Feb 15, 2016 at 9:08 AM, Matt Sicker <[email protected]> wrote: >>>>> >>>>>> Sounds good. Remko was mentioning how they could be combined by >>>>>> checking if get() returns an instanceof Message, but using the wildcard >>>>>> bounds like you show sounds like a better way (where possible). >>>>>> >>>>> >>>>> Wasn't Remko talking about a different case? >>>>> >>>>> Right now we have: >>>>> >>>>> traceExit(MessageSupplier, R) >>>>> traceExit(Supplier<? extends Message>, R) >>>>> >>>>> Both "MessageSupplier.get()" and "Supplier<? extends Message>" *do* >>>>> return a Message so I think we can remove traceExit(Supplier<? extends >>>>> Message>, R) (which is not even used/tested ATM). >>>>> >>>> >>>> Ah, but we do not have a traceExit(Supplier<R>) >>>> >>>> I'll add that completeness, and also traceExit(EntryMessage). My use >>>> cases only call for traceExit(EntryMessage, R) and traceExit(EntryMessage). >>>> >>>> Gary >>>> >>>> >>>>> Gary >>>>> >>>>>> >>>>>> On 15 February 2016 at 11:06, Gary Gregory <[email protected]> >>>>>> wrote: >>>>>> >>>>>>> Since 2.4 we have: >>>>>>> >>>>>>> public interface MessageSupplier { >>>>>>> >>>>>>> /** >>>>>>> * Gets a Message. >>>>>>> * >>>>>>> * @return a Message >>>>>>> */ >>>>>>> Message get(); >>>>>>> } >>>>>>> >>>>>>> and >>>>>>> >>>>>>> public interface Supplier<T> { >>>>>>> >>>>>>> /** >>>>>>> * Gets a value. >>>>>>> * >>>>>>> * @return a value >>>>>>> */ >>>>>>> T get(); >>>>>>> } >>>>>>> >>>>>>> Which smells fishy to me. Instead I propose: >>>>>>> >>>>>>> public interface MessageSupplier extends Supplier<Message> { >>>>>>> // empty >>>>>>> } >>>>>>> >>>>>>> Whether or not we do the above, we can replace: >>>>>>> >>>>>>> traceExit(R, Supplier<? extends Message>) >>>>>>> >>>>>>> with: >>>>>>> >>>>>>> traceExit(R, MessageSupplier) >>>>>>> >>>>>>> which we already have. >>>>>>> >>>>>>> A test search shows the above as the only instance of "Supplier<? >>>>>>> extends Message>" in our Java files. >>>>>>> >>>>>>> Thoughts? >>>>>>> >>>>>>> -- >>>>>>> E-Mail: [email protected] | [email protected] >>>>>>> Java Persistence with Hibernate, Second Edition >>>>>>> <http://www.manning.com/bauer3/> >>>>>>> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/> >>>>>>> Spring Batch in Action <http://www.manning.com/templier/> >>>>>>> Blog: http://garygregory.wordpress.com >>>>>>> Home: http://garygregory.com/ >>>>>>> Tweet! http://twitter.com/GaryGregory >>>>>>> >>>>>> >>>>>> >>>>>> >>>>>> -- >>>>>> Matt Sicker <[email protected]> >>>>>> >>>>> >>>>> >>>>> >>>>> -- >>>>> E-Mail: [email protected] | [email protected] >>>>> Java Persistence with Hibernate, Second Edition >>>>> <http://www.manning.com/bauer3/> >>>>> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/> >>>>> Spring Batch in Action <http://www.manning.com/templier/> >>>>> Blog: http://garygregory.wordpress.com >>>>> Home: http://garygregory.com/ >>>>> Tweet! http://twitter.com/GaryGregory >>>>> >>>> >>>> >>>> >>>> -- >>>> E-Mail: [email protected] | [email protected] >>>> Java Persistence with Hibernate, Second Edition >>>> <http://www.manning.com/bauer3/> >>>> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/> >>>> Spring Batch in Action <http://www.manning.com/templier/> >>>> Blog: http://garygregory.wordpress.com >>>> Home: http://garygregory.com/ >>>> Tweet! http://twitter.com/GaryGregory >>>> >>>> >>> >>> >>> -- >>> Matt Sicker <[email protected]> >>> >> >> > -- E-Mail: [email protected] | [email protected] Java Persistence with Hibernate, Second Edition <http://www.manning.com/bauer3/> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/> Spring Batch in Action <http://www.manning.com/templier/> Blog: http://garygregory.wordpress.com Home: http://garygregory.com/ Tweet! http://twitter.com/GaryGregory
