On Tue, 15 Mar 2011, Robbie Gemmell wrote:
On 15 March 2011 17:33, Rajith Attapattu <[email protected]> wrote:
On Tue, Mar 15, 2011 at 1:24 PM, Robert Godfrey <[email protected]>wrote:
I see both side here, I think the fundamental issue for me is that I
hadn't previously picked up that when the change to the default addressing
syntax was changed in the 0.8 release, the tests were made to explicitly run
with the Binding URL as the default. As such we are not exposing the
"default" that we are giving to users when we run the tests - thus the
expected normal code path is not being exercised in our tests other than in
the one test that Robbie highlighted (and this uses the explicit "ADDR:"
syntax).
As such I don't feel like simply running the tests gives me the confidence
that this change will not have unforseen consequences. In truth I actually
now feel quite nervous about what we did in 0.8. I strongly feel that we
should update the tests to use the default (which is now ADDR) syntax -
changing any test where the BindingURL is implicitly expected.
Given that this is a "change" rather than a "bug fix", the fact that we
are supposed to be in code freeze in preparation for release, and that I
don't have confidence that we are adequately testing the "default" behaviour
using the standard tests - I don't feel that it is appropriate to include in
0.10.
I actually consider this a bug as I didn't really implement what I should
have in the first place.
The current behaviour is neither backwards compatible nor correct as per
the JMS spec.
As for your comments about the tests, I fully agree with you and have said
the same in my response to Robbie with some examples.
But as I mentioned, that risk is **already there in 0.10** and this commit
is not adding anything to it.
Rajith
In my mind this is a 'feature' change rather than bug fix as the patch would
alter a behaviour of the client that was, correctly or otherwise, long since
put there deliberately and which you preserved deliberately.
You are correct that there is existing risk already in 0.10, however this is
an additional change which would clearly introduce an additional risk of
unforseen effects in its own right versus not including it. Not having
enough confidence in the existing situation is no reason to throw caustion
to the wind.
Robbie
Thanks, Rob, Robbie, and Rajith, for the discussion.
Applying the "no regressions, even prospective ones" rule, I oppose this
change for 0.10.
To my impression, it blurs the feature/bug distinction, and I personally
accept it as a defect. That doesn't mean, however, that we should accept
it for 0.10. Since the fix also constitutes an important behavior change,
and since it poses a real risk of introducing new regressions, I think it
should wait for 0.12.
Justin
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:[email protected]
