Hi,
I made some changes according to your review comments. Please take a quick
look at this new patch and approve this WL.
http://lists.mysql.com/commits/51994
Please see my other comments inline.
Warm Regards,
Hema Sridharan
QA Developer
Sun Microsystems / Mysql Inc, www.sun.com
www.mysql.com
Office: Austin TX 78728, USA
Are you MySQL certified? www.mysql.com/certification
> -----Original Message-----
> From: Jorgen.Loland@stripped [mailto:Jorgen.Loland@stripped]
> Sent: Tuesday, August 19, 2008 3:23 AM
> To: 'Hema Sridharan'
> Cc: Chuck Bell; commits@stripped
> Subject: Re: bzr commit into mysql-6.0-backup branch
> (hema:2683) WL#4227
>
> Chuck Bell wrote:
> > Hema,
> >
> > I think this patch should move the backup_charsets test to the new
> > backup_charset suite.
Hema: I will move this test when I move all the backup tests to the suite.
> >
> > Patch looks good so far. Just a few comments. Most are general and
> > apply to the entire patch (things to watch out for mainly).
>
> Hema,
>
> The patch looks good. In addition to Chucks comments, I only
> want to add one requirement before approving:
>
> Requirement:
> 1. Bug#33566 has been fixed (pushed to backup branch
> yesterday!). The disabled falcon partition tests should be enabled.
Hema: Agreed, will enable it.
>
> Suggestion:
> I find it hard to stay focused when reviewing a 5500 LOC
> patch. I have mainly concentrated on checking that the test
> files are sound; if there are inconsistencies in the result
> files I may have overlooked them.
> Whenever possible, I would prefer multiple smaller patches
> than one gigantic one. I don't think there should be any
> problem in pushing multiple patches for the same wl.
Hema: I will try to make smaller test cases from next time onwards.
Thanks,
Hema.S
>
> --
> Jørgen Løland
>
> --
> MySQL Code Commits Mailing List
> For list archives: http://lists.mysql.com/commits
> To unsubscribe: http://lists.mysql.com/commits?unsub=1
>
>
