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
