Looks much better now. --emi
Pe 8 sept. 2017, la 12:04, UBIK LOAD PACK Support <[email protected]> a scris: > Hello Emilian, > There was indeed one last issue in TCPClientImpl with backward > compatibility, we pushed a commit > It should be ok now, but please review ( > https://github.com/apache/jmeter/pull/306/files): > > - If AbstractTCPClient implement the old method they will work > - If AbstractTCPClient implement the new method, they will of course > work > - If 3rd party code calls the old method, they should also work > > > Regards > > @ubikloadpack Team >> On Fri, Sep 8, 2017 at 6:31 AM, Emilian Bold <[email protected]> wrote: >> >> It is backwards compatible as an SPI, meaning that 3rd party >> implementations that use AbstractTCPClient will still work if they >> implement only the old method. >> >> But it is not backwards compatible as an API because 3rd party code that >> calls the old method will now receive nulls for those existing >> implementations. >> >> So the question is if this is supposed to be an API or it's just an SPI. >> >> --emi >> >> On Fri, Sep 8, 2017 at 12:03 AM, Philippe Mouawad < >> [email protected]> wrote: >> >>> Hi Emilian >>> AFAIU, it is backward compatible no ? >>> >>> in AbstractTCPClient.java >>> <https://github.com/apache/jmeter/pull/306/files#diff-37500f >>> a9c4a5285efe7b85e9bcd62614>, >>> the new method calls the old one. >>> >>> Only the subclasses has been modified to implement the new one. >>> >>> Thanks for reviewing >>> >>> On Thu, Sep 7, 2017 at 10:58 PM, Emilian Bold <[email protected]> >>> wrote: >>> >>>> I don't know the codebase but why bother marking the old method as >>>> @Deprecated if you are not making the change backwards compatible? >>>> >>>> I see 'return null' in two existing methods while previously those >>> methods >>>> returned something. >>>> >>>> A proper pattern would have been to call read(is, null) or read(is, new >>>> DummySampleResult()). >>>> >>>> >>>> --emi >>>> >>>>> On Thu, Sep 7, 2017 at 11:25 PM, pmouawad <[email protected]> wrote: >>>>> >>>>> Github user pmouawad commented on the issue: >>>>> >>>>> https://github.com/apache/jmeter/pull/306 >>>>> >>>>> Hello >>>>> Unless there is a NO GO , I'll commit this tomorrow. >>>>> >>>>> Thanks >>>>> >>>>> >>>>> --- >>>>> >>>> >>> >>> >>> >>> -- >>> Cordialement. >>> Philippe Mouawad. >>> >>
