← Back to team overview

deja-dup-team team mailing list archive

Re: [Merge] lp:~urbans/deja-dup/deja-dup.nautilus into lp:deja-dup

 

On Aug 5, 2010, at 10:22 PM, Michael Terry wrote:

> Looks like you can drop changes in common/OperationRestore.vala.
> There are warnings in AssistantDirectoryHistory about unused  
> variables.

Check

> Drop the "stdout.printf("\n\ndirhistmode\n\n"); line.
>
> The interface files need some extra support.  They should be moved  
> to a subdirectory of data/ and added to data/Makefile.am, to be  
> installed in pkgdatadir.  Then they should be searched for in  
> g_get_system_data_dirs().  And finally, the testing framework needs  
> to be able to find them in the local directory (just by adjusting  
> XDG_DATA_DIRS if it doesn't already).

Implemented by looking at g_get_system_data_dirs and checking if there  
exists a deja-dup/interfaces/[glade file] in it.

Note: Due to issues with const that are non-trivial to fix, compiler  
issues the following warning about g_get_system_data_dirs:
AssistantDirectoryHistory.c: In function  
‘assistant_directory_history_get_glade_file’:
AssistantDirectoryHistory.c:668: warning: assignment from incompatible  
pointer type

Basically this warning is out of our domain because it's an issue with  
vala itself. Sometime in the future Vala should fix const issues and  
the warning will go away (or so I am told).

>
> When testing, I'm seeing some errors like:
>
> (deja-dup:10259): Gtk-CRITICAL **: IA__gtk_icon_theme_load_icon:  
> assertion `icon_name != NULL' failed

Resolved by setting restore icon.

>
> ** (deja-dup:10262): CRITICAL **:  
> deja_dup_operation_continue_with_passphrase: assertion `self !=  
> NULL' failed

Resolved. The issue was that duplicity returns collection-status data  
twice and on call the instance of OperationStatus has already been  
cleared (at least I understand it that way).

Resolved by moving do_query_files_at_date into condition of if  
(this.backups_queue_filled) in handle_collection_dates.

>
> This second one, you were able to fix by commenting out that  
> continue_with_passphrase line.  But something is wrong if that's the  
> fix.  I think you should look at why continue_with_passphrase is  
> being called on a null instance.

Indeed, it was my mistake to actually interpret that as a solution. I  
think I fixed it properly now.

>
> I'd like --dir-history renamed to --restore-missing, so it's clear  
> that it's about restoring and what it restores.

Done, also added a forgotten check if file exists or is even directory.

>
> I'm OK with leaving "Done!", but let's rename it to "Scanning  
> finished" or something equally boring.

Done

>
> About "this.op = null" vs "op = null", it was an issue because there  
> was also a local variable (in the function arguments) called "op".   
> So "op" refers to the local variable, and "this.op" to the class  
> variable.

Ooops, didn't notice that. I am surprised compiler didn't begin to  
complain about this...

>
> I'd still like to see a test (that handles both missing files and  
> missing folders) added for this feature to the test suite.  And we  
> still need nautilus UI for it.  Those can come after the above fixes  
> though.
>
> And remember to add yourself to the About dialog (MainWindow.vala)  
> and the AUTHORS file.
> -- 
> https://code.launchpad.net/~urbans/deja-dup/deja-dup.nautilus/+merge/ 
> 29723
> You are the owner of lp:~urbans/deja-dup/deja-dup.nautilus.

-- 
https://code.launchpad.net/~urbans/deja-dup/deja-dup.nautilus/+merge/29723
Your team Déjà Dup Maintainers is subscribed to branch lp:deja-dup.



References