Some comments and questions:
- conflict in xtrabackup.cc
- typo in the comments for cleanup_dir_recursively() (double "directory")
- wrong bug reference in the test case comments
- there's no need for "\/+" in the following code:
- can you clarify the purpose of the code in lines 50-52?
- error handling for unlink() would be nice
- did you take the comment in lines 125-128 into account?
- a few lines are longer than 80 chars, try:
: bzr diff -c-1 | grep -0 '^+' | expand | awk 'length > 80'
or better yet, try to use a good text editor that can be configured
to highlight such lines
- the check_newer argument is not used. does it really make sense to
add some _new_ code to support it? Wouldn't it be better to get rid
of it?
- in the test, do we really have to restart the server between full
and incremental backup, or it's just a copy-paste from another test?
Sergei,
Some comments and questions: dir_recursively () (double "directory")
- conflict in xtrabackup.cc
- typo in the comments for cleanup_
- wrong bug reference in the test case comments
- there's no need for "\/+" in the following code:
:+ (my $rel_path = $File::Find::name) =~ s/^$copy_ dir_src\ /+//;
- can you clarify the purpose of the code in lines 50-52?
- error handling for unlink() would be nice
- did you take the comment in lines 125-128 into account?
- a few lines are longer than 80 chars, try:
: bzr diff -c-1 | grep -0 '^+' | expand | awk 'length > 80'
or better yet, try to use a good text editor that can be configured
to highlight such lines
- the check_newer argument is not used. does it really make sense to
add some _new_ code to support it? Wouldn't it be better to get rid
of it?
- in the test, do we really have to restart the server between full
and incremental backup, or it's just a copy-paste from another test?