Jørgen Løland wrote:
> Øystein,
>
> Patch is good to push, pending a few bugs in the Norwegian error
> messages. There is no need for a new review after fixing these. I have
> verified that all tests passed on Ubuntu x86
>
> Requests
> --------
> 1.
> This norwegian message is not complete:
> + norwegian-ny "Sendinga av svar til klienten etter %-.64s"
>
> 2.
> Will these strange characters translate into 'ø'?
> + nor "Backup/Restore: Feil ved lukning av backup-strøm"
>
> 3.
> Are keywords supposed to be upper case in Norwegian error messages as well?
> + eng "Error on accessing binlog during BACKUP"
> + nor "Lesing av binlog feilet under backup"
Maybe I should just leave localization to the localization experts! I
do think the character issue is an email issue, and I think it will
not be an issue when using bazaar.
>
>
> Suggestions
> -----------
> 1. I think 5 lines of if-conditions makes the code more complicated than
> necessary.
>
> @@ -317,14 +307,16 @@ Backup_info::Backup_info(Backup_restore_
> + if (hash_init(&ts_hash, &::my_charset_bin, 16, 0, 0,
> + Ts_hash_node::get_key, Ts_hash_node::free, MYF(0))
> + ||
> + hash_init(&dep_hash, &::my_charset_bin, 16, 0, 0,
> + Dep_node::get_key, Dep_node::free, MYF(0)))
> + {
I am not sure I agree. With the given lay-out, I think it is pretty
obvious what is happening here. I guess the alternatives is to
duplicate the error handling code and handle each hash_init in
separate if-statements. I am not sure that is necessarily easier to
follow.
--
Øystein
>
> Comments
> --------
> Good work!
>
>
> Oystein.Grovlen@stripped wrote:
>> #At file:///home/oysteing/mysql/mysql-6.0-backup-1/
>>
>> 2700 oystein.grovlen@stripped 2008-10-02
>> BUG#39089/WL#4384 Fix missing error handling in backup code.
>> Make sure errors returned from functions are reports are handled
>> and/or logged.
>> modified:
>> sql/Makefile.am
>> sql/backup/backup_aux.h
>> sql/backup/backup_info.cc
>> sql/backup/backup_kernel.h
>> sql/backup/data_backup.cc
>> sql/backup/image_info.cc
>> sql/backup/image_info.h
>> sql/backup/kernel.cc
>> sql/backup/logger.cc
>> sql/backup/logger.h
>> sql/backup/stream.cc
>> sql/backup/stream.h
>> sql/mdl.cc
>> sql/share/errmsg.txt
>