Chuck Bell wrote:
> Hema,
>
> I think this patch should move the backup_charsets test to the new
> backup_charset 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.
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.
--
Jørgen Løland
