On Tue, Mar 8, 2011 at 3:33 PM, Rajith Attapattu <[email protected]> wrote:
> > > On Tue, Mar 8, 2011 at 3:01 PM, Robbie Gemmell > <[email protected]>wrote: > >> Done. To summarise here though: >> >> 1. QPID-3109: looks good, although as mentioned on the JIRA a second >> commit >> http://svn.apache.org/viewvc?view=rev&rev=1079059 is also required here, >> to >> enable compilation to succeed. >> > > There is test case for this at > http://svn.apache.org/viewvc?view=revision&revision=1076670 > All though I agree that it would be good to add a test case to check close > on a producer created with a null destination. > However that functionality is being tested in another test case that I have > put in place to test bug that I am currently working on. > So btw the two test cases I think I have covered the bases. > Due to some in-progress local changes the compilation issue wasn't detected > on my local. Sorry about that. > > >> >> 2. & 3. QPID-2732: changes look reasonable. >> >> 4. This specific change looks good to restore the prior behaviour, though >> in >> general I wouldn't consider work around change of accept mode to be very >> low >> risk. I would suggest there should have been tests added to verify the >> behaviour of the original change that caused the issue though (same goes >> for >> all really). >> >> I have a test in place for the reliability modes to ensure that the string > is passed properly and the exception is thrown for the unsupported > combination. > But I am also trying to see if I can enhance the test to see if I can test > if the subscription is created with the correct accept-mode. So far it > hasn't been easy to get that part going. > So the tests are on the way. > > Further to the above comment, All though I don't have an automated test to work out if the accept-mode is set properly the changes are verified manually using logging. Similarly Andrew Kennedy has also tested the change he made. So I am quite confident about the changes. > Robbie >> >> -----Original Message----- >> From: Justin Ross [mailto:[email protected]] >> Sent: 08 March 2011 18:08 >> To: [email protected] >> Subject: Re: Request for the following commits to be included in 0.10 >> >> Hi, Rajith. I approved item 4. >> >> Robbie, would you indicate briefly up or down for 0.10 in the comments of >> qpid-3109 and -2732? >> >> Justin >> >> On Tue, 8 Mar 2011, Rajith Attapattu wrote: >> >> > Hi Justin, >> > >> > I'd like the following commits to be included in the 0.10 release >> branch. >> > >> > 1. http://svn.apache.org/viewvc?rev=1078961&view=rev - QPID-3109 - >> > Fairly low risk. >> > This is a simple bug fix. >> > >> > 2. http://svn.apache.org/viewvc?rev=1078971&view=rev - QPID-2732 - >> > Low risk. >> > This is an addition to the initial commit made under this JIRA. >> > It adds default reliability modes and throws an exception for an >> > unsupported combination. >> > >> > 3. http://svn.apache.org/viewvc?rev=1079408&view=rev - QPID-2732 - Low >> Risk >> > The reliability mode is used to mark a transfer unreliable and that >> > flag is used to determine if a transfer should be stored for replay or >> not. >> > Again a fairly straightforward and low risk commit. However I have >> > asked Rafi to have a quick look at it as well. >> > >> > 4. http://svn.apache.org/viewvc?rev=1079402&view=rev - QPID-3127 - >> > Very low risk. >> > Fixes a bug in the initial commit for QPID-2732. >> > It just sets accept mode to NONE for NO_ACKNOWLEDGE. >> > >> > Regards, >> > >> > Rajith >> > >> >> --------------------------------------------------------------------- >> Apache Qpid - AMQP Messaging Implementation >> Project: http://qpid.apache.org >> Use/Interact: mailto:[email protected] >> >> >> >> --------------------------------------------------------------------- >> Apache Qpid - AMQP Messaging Implementation >> Project: http://qpid.apache.org >> Use/Interact: mailto:[email protected] >> >> >
