jmeter-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Emilian Bold <emilian.b...@gmail.com>
Subject Re: [GitHub] jmeter issue #306: Fix to BUG_60156
Date Fri, 08 Sep 2017 09:28:45 GMT
Looks much better now.

--emi

Pe 8 sept. 2017, la 12:04, UBIK LOAD PACK Support <support@ubikloadpack.com> 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 <emilian.bold@gmail.com> 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 <
>> philippe.mouawad@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.bold@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 <git@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.
>>> 
>> 

Mime
View raw message