Code review comment for lp:~rockstar/entertainer/dialog-magics

Revision history for this message
Samuel Buffet (samuel-buffet) wrote :

Paul,

Not a lot of things to say except that everything looks good.

I've spent some time toying with it and it was lovely working.

Test are okay.
Lint gives me some issues but as you're actively hacking on future, you have time to fix those.

Thanks for the work,
Samuel-

A few details below :

=== modified file 'cfg/content.conf'
--- cfg/content.conf 2009-05-31 17:11:16 +0000
+++ cfg/content.conf 2009-07-14 01:00:37 +0000
 metric_units = True

> Maybe you can remove that "metric_units" it's deprecated.

=== modified file 'entertainerlib/dialog.py'
--- entertainerlib/dialog.py 2009-06-30 01:06:01 +0000
+++ entertainerlib/dialog.py 2009-07-14 00:56:36 +0000
@@ -50,33 +50,25 @@

     def on_url_dialog_ok_button_clicked(self, widget):
@@ -555,9 +434,9 @@
         """
         self.on_location_find_button_clicked(widget)

- def on_button_video_rebuild_clicked(self, widget):
+ def on_button_media_rebuild_clicked(self, widget):
         """
- Rebuild video cache requested
+ Rebuild media cache requested
         @param widget: GTK-Widget
         """
> Can you also remove the @param here and place everything on one line.
> Thanks.

=== modified file 'entertainerlib/tests/test_configuration.py'
--- entertainerlib/tests/test_configuration.py 2009-06-30 01:06:01 +0000
+++ entertainerlib/tests/test_configuration.py 2009-07-14 01:04:31 +0000
@@ -108,11 +108,6 @@
         '''Test getting the slideshow step'''
         self.assertEqual(self.configuration.get_slideshow_step(), 5)

- def test_hidden_files_folders(self):
- """Test getting the `display_hidden_files_folders` setting."""
- self.assertEqual(self.configuration.display_hidden_files_folders(),
- False)
-

> Why did you keep the setting in content-conf and removed the test?

review: Approve

« Back to merge proposal