List:Commits« Previous MessageNext Message »
From:Øystein Grøvlen Date:October 14 2008 11:23am
Subject:Re: bzr commit into mysql-6.0-backup branch (oystein.grovlen:2700)
Bug#39089 WL#4384
View as plain text  
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
> 

Thread
bzr commit into mysql-6.0-backup branch (oystein.grovlen:2700)Bug#39089 WL#4384Oystein.Grovlen2 Oct
  • Re: bzr commit into mysql-6.0-backup branch (oystein.grovlen:2700)Bug#39089 WL#4384Jørgen Løland13 Oct
    • Re: bzr commit into mysql-6.0-backup branch (oystein.grovlen:2700)Bug#39089 WL#4384Øystein Grøvlen14 Oct