Code review comment for lp:~azzar1/unity-lens-files/add-filesystem-icons

Revision history for this message
Paweł Stołowski (stolowski) wrote :

Hi Andrea, nice job, it works well and looks good, however it's missing support for recently introduced previews - if you right-click on a device icon in the dash, an empty preview is displayed and an error is printed to the stderr:

(process:3255): unity-files-daemon-WARNING **: daemon.vala:1042: Couldn't stat file: 'device://099A-74A7'

Could you please check with design what would they like to see as a device preview and implement the missing bits in the preview(..) method?

Manual tests should now follow the format requested by QA - see TEST_TEMPLATE.txt in unity source code. I know existing tests don't follow it (and need to be fixed), but when we introduce new tests file we should stick to the new format.

Also some minor comments:

This can fail and throw GLib.Error:
415 + AppInfo.launch_default_for_uri (get_volume_uri (), null);

Formatting issues (spaces missing):
363 + public Volume volume {get; set construct; }

378 + Object (volume:volume, uri:"device://"+uuid, icon:icon_name, display_name:name, dnd_uri:"device://"+uuid);

review: Needs Fixing

« Back to merge proposal