[Box Backup] Code in SVN -- review notes

[Ben Summers]

On 1 Nov 2005, at 06:26, Jonathan Morton wrote:

>> chromi/diffopt
>> ==============
>> static int sMaximumDiffTime = 600;             // maximum time to  
>> spend diffing
>>     -- not sure whether this is good or not?
>>
>
> It helps to find more matching sections when working with large,  
> complex files, which tends to save bandwidth and may allow more  
> past revisions to remain stored.  The time can always be overridden  
> by the configuration file - this just adjusts the default.

I would prefer the default was small, but overridden in the default  
config file to be large.

>
> Something I noticed on OSX, though - is it really wall-time or CPU- 
> time being measured for that?  Unless/until a keep-alive is set up  
> for the connection, wall-time is necessary in order to reliably  
> avoid timeouts.  (Or, I have a more invasive revision idea to the  
> diff/recipe code, which would solve about three problems at once -  
> the timeouts included.)

It should be elapsed time (wall-time?), not CPU time, for precisely  
this reason. Unless I've made a mistake.

>
>
>> Style: A tendency to place {'s on the same line as ifs and whiles.  
>> And "} else {"...
>>
>
> I thought I'd caught all of those.  What did I miss?

lib/backupclient/BackupStoreFileDiff.cpp, all over the place.

Other people's coding styles are just so _inconsiderate_... :-)


>
>
>> Is docs/backup/encrypt_rsync.txt changed, or just renamed? Use svn  
>> rename first anyway.
>>
>
> Both, I think.  I did use the appropriate SVN command to rename it,  
> but this might not show in 'svn diff'.

That would be it.

Ben