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 < philippe.moua...@gmail.com> 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 <emilian.b...@gmail.com> > 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 <g...@git.apache.org> 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. >
