Hi, Saurabh, Thanks again for your submissions. I've reviewed your two patches and have these comments:
1. The patches that you attached to the issues do not correspond to the defects that the issues address: * The patch attached to issue 1587 (https://mifos.dev.java.net/issues/show_bug.cgi?id=1587) actually contains revisions to fix issue 1590. * The patch attached to issue 1590 (https://mifos.dev.java.net/issues/show_bug.cgi?id=1590) actually contains revisions to fix issue 1592. After considering the comments that follow, please attach the correct patches to the correct issues. That will help facilitate our review quite a bit. 2. The patch for issue 1590 (that is attached to issue 1587) does fix that defect. However, I cannot commit it because the patch depends on revisions submitted for issue 1592 (attached to issue 1590). Specifically, you have included revisions to the class Constants.java in the 1592 patch but did not include them in the 1590 patch. 3. The patch for issue 1592 (that is attached to issue 1590) does not correct the defect. The drop-down box that should display acceptable payment types for fee payment still incorrectly displays the acceptable types for loan disbursement. I look forward to your resubmission of these patches. Also, did you plan to submit a patch for the closely related issue 1587? Keith Pierce On Feb 10, 10:11 pm, "Aliya Walji" <[EMAIL PROTECTED]> wrote: > Hi Saurabh, > > Thanks so much for attaching the patches to the issue. Just a quick > reminder to you and the rest of the community around the process, please > make sure to also add the "Patch' keyword to the item in the issue > tracker so that we can keep track of items in the issue tracker that are > awaiting code review and check-in. I've added the keyword for the two > items below. > > Thanks, > > Aliya > > ________________________________ > > From: [EMAIL PROTECTED] > [mailto:[EMAIL PROTECTED] On Behalf Of > Saurabh Kumar > Sent: Thursday, February 07, 2008 3:35 AM > To: Developer > Subject: Re: [Mifos-developer] Patch for defects 1587,1590 and > suggestion ......... > > Hi Aliya, > > I have attached the patches in the issue tracker. > > https://mifos.dev.java.net/issues/show_bug.cgi?id=1587 > > https://mifos.dev.java.net/issues/show_bug.cgi?id=1590 > > Thanks & Regards, > > P Think before you print > > Saurabh Kumar * Developer * SunGard * Technology Services * Divyasree > Chambers, Langford Road, Bangalore 560025 India > Tel+91-80-2222-0501* Mobile+91-9886945575* Fax +91-80-2222-0511 > *www.sungard.com<http://www.sungard.com/> > > -----Original Message----- > From: [EMAIL PROTECTED] > > [mailto:[EMAIL PROTECTED] On Behalf Of > Aliya Walji > Sent: Monday, February 04, 2008 10:37 PM > To: Developer > Subject: Re: [Mifos-developer] Patch for defects 1587,1590 and > suggestion ......... > > Hi Saurabh, > > Can you attach the patches to the respective bugs in the issue tracker? > This is our new process for tracking patches that need to be reviewed. > > I'll let a developer comment on your suggestion below :-) > > Thanks, > > Aliya > > ________________________________ > > From: [EMAIL PROTECTED] > [mailto:[EMAIL PROTECTED] On Behalf Of > Saurabh Kumar > Sent: Monday, February 04, 2008 2:21 AM > To: [EMAIL PROTECTED] > Subject: [Mifos-developer] Patch for defects 1587,1590 and suggestion > ......... > > Hi, > > Please find the patch for defects 1587 and 1590. > > Please note that apart from bug fixing I have removed some hard coding > of strings from code. I have added constants for these strings in > Constants.java file. > > 1587 >> Define Accepted payment methods not working for clients/groups > > https://mifos.dev.java.net/issues/show_bug.cgi?id=1587 > > 1590 >> Define accepted payment methods not separated for savings > deposits/withdrawals > > https://mifos.dev.java.net/issues/show_bug.cgi?id=1590 > > Suggestion >> > > I have noticed that at many places we are using hard coding for empty > string and some common strings like "savings", "loan", etc. Also in > getSecurity() method of all the action classes we are using hard coded > values for strings like "load", "preview", "previous". We can move all > these constants to Constants.java file or to the constants files of > there respective modules. We can voluntarily take up this task while > fixing bugs, i.e, while fixing bugs we can remove hard coding for common > strings from the corresponding action classes. > > Thanks & Regards, > > P Think before you print > > Saurabh Kumar * Developer * SunGard * Technology Services * Divyasree > Chambers, Langford Road, Bangalore 560025 India > Tel+91-80-2222-0501* Mobile+91-9886945575* Fax +91-80-2222-0511 > *www.sungard.com<http://www.sungard.com/> > > > > ------------------------------------------------------------------------- > This SF.net email is sponsored by: Microsoft > Defy all challenges. Microsoft(R) Visual Studio > 2008.http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
