Code review comment for lp:~rockstar/entertainer/kill-fixmes

This branch is also merged with *more-pylint-fun*.

So as long as *more-pylint-fun* is okay I approve this one as well.

Paul, thanks to clean the bug tracker and remove all that.

here is a diff against *more-pylint-fun*

=== modified file 'entertainerlib/backend/components/mediacache/image_cache.py'
--- entertainerlib/backend/components/mediacache/image_cache.py 2009-02-07 22:12:25 +0000
+++ entertainerlib/backend/components/mediacache/image_cache.py 2009-02-08 20:31:58 +0000
@@ -107,8 +107,6 @@
         if self.isFileInCache(filename):
             self.removeFile(filename)
             self.addFile(filename)
- #FIXME: This is maybe too expensive method for this purpose!
- # Try to optimise it.

     def addDirectory(self, path):
         """
@@ -165,7 +163,6 @@
         """
         self.removeDirectory(path)
         self.addDirectory(path)
- #FIXME: This is very expensive method to call! Try to optimise it.

     def isFileInCache(self, filename):
         """Check if file is already in cache. Returns boolean value."""
@@ -250,12 +247,10 @@
             a_title = path[path.rfind('/')+1:].replace('_',' ').title()
             a_description = ""

- #FIXME: If not found we could generate fancy album thumb
         if os.path.exists(album_thumb):
             thumbnailer = ImageThumbnailer(album_thumb)
             thumbnailer.create_thumbnail()
             a_hash = thumbnailer.get_hash()
- del thumbnailer #FIXME: Does this make any sense?
         else:
             a_hash = ""

=== modified file 'entertainerlib/backend/components/mediacache/video_cache.py'
--- entertainerlib/backend/components/mediacache/video_cache.py 2009-02-07 22:12:25 +0000
+++ entertainerlib/backend/components/mediacache/video_cache.py 2009-02-08 20:31:58 +0000
@@ -239,7 +239,6 @@
         thash = thumbnailer.get_hash()
         del thumbnailer

- #FIXME: read resolution, length etc. from video file and add to db
         self.__db_cursor.execute("""INSERT INTO videofile(filename, hash)
                                     VALUES (:fn, :hash)""",
                                     { 'fn': filename, 'hash': thash, } )

=== modified file 'entertainerlib/frontend/gui/screens/photo_albums.py'
--- entertainerlib/frontend/gui/screens/photo_albums.py 2009-02-07 21:43:35 +0000
+++ entertainerlib/frontend/gui/screens/photo_albums.py 2009-02-08 20:31:58 +0000
@@ -106,7 +106,7 @@
                 nro_of_photos = str(nro_of_photos)
             item = TextMenuItem(0.4393, 0.0781, album.get_title(),
                 nro_of_photos)
- item.set_userdata(album) #FIXME: Should we use URLs as KEYS?
+ item.set_userdata(album)
             self.menu.add_actor(item)

         self.add(self.menu)

=== modified file 'entertainerlib/frontend/gui/screens/video_osd.py'
--- entertainerlib/frontend/gui/screens/video_osd.py 2009-02-03 23:37:57 +0000
+++ entertainerlib/frontend/gui/screens/video_osd.py 2009-02-08 20:31:58 +0000
@@ -212,8 +212,6 @@
         if event_type == UserEvent.PLAYER_STOP:
             self.display_progress_bar(hide_after_delay=False)
             self.pause_texture.hide()
- #FIXME
- print "CREATE VIDEO STOPPED SCREEN HERE"

         # Video aspect ratio changed. Display aspect ratio logo on screen
         elif event_type == UserEvent.USE_ASPECT_RATIO_1:

=== modified file 'entertainerlib/frontend/gui/widgets/grid_menu.py'
--- entertainerlib/frontend/gui/widgets/grid_menu.py 2009-02-06 08:13:48 +0000
+++ entertainerlib/frontend/gui/widgets/grid_menu.py 2009-02-08 20:31:58 +0000
@@ -202,14 +202,12 @@
         """
         Override clutter.Group method. Returns number of menuitems.
         """
- #FIXME: Is this safe?
         return len(self.items)

     def get_nth_child(self, index):
         """
         Override clutter.Group method. Returns one menuitem.
         """
- #FIXME: Is this safe?
         return self.items[index]

     def get_number_of_items(self):
@@ -522,7 +520,6 @@
         # BELOW IS ANIMATION CODE FOR MENU CURSOR. SHOULD CURSOR BE ANIMATED?
 # if self.animate:
 # # Finish previous animation before new
-# # FIXME: Doesn't seem to work as it should
 # if self.cursor_timeline is not None and (
 # self.cursor_timeline.is_playing():
 # self.cursor_timeline.pause()
@@ -585,7 +582,6 @@
 # y = self.itemgroup.get_y()
 #
 # # Finish previous animation before new
-# # FIXME: Doesn't seem to work as it should
 # if self.content_timeline is not None and (
 # self.content_timeline.is_playing():
 # self.content_timeline.pause()

=== modified file 'entertainerlib/frontend/gui/widgets/menu.py'
--- entertainerlib/frontend/gui/widgets/menu.py 2008-11-19 21:50:24 +0000
+++ entertainerlib/frontend/gui/widgets/menu.py 2009-02-08 20:31:58 +0000
@@ -191,7 +191,6 @@
         """
         # Set current menuitem
         self.__items[self.__current].set_active(False)
- #FIXME: What if there is not that many?
         self.__current = self.__current + 8
         self.__items[self.__current].set_active(True)

=== modified file 'entertainerlib/frontend/media_player.py'
--- entertainerlib/frontend/media_player.py 2009-01-08 15:38:26 +0000
+++ entertainerlib/frontend/media_player.py 2009-02-08 20:31:58 +0000
@@ -235,7 +235,6 @@
         """
         if self.playlist is not None:
             if self.shuffle:
- # FIXME: Should we remember order of randomly played tracks
                 self.set_media(self.playlist.get_random(), True)
             elif self.playlist.has_previous():
                 self.set_media(self.playlist.get_previous(), True)

=== modified file 'entertainerlib/frontend/medialibrary/music.py'
--- entertainerlib/frontend/medialibrary/music.py 2009-02-07 21:43:35 +0000
+++ entertainerlib/frontend/medialibrary/music.py 2009-02-08 20:31:58 +0000
@@ -262,7 +262,6 @@
         self.title = title
         self.total_length = 0

- #FIXME: Album needs to be refactored to have no dependence on the db.
         if not cursor:
             self.db_connection = sqlite.connect(self.config.MUSIC_DB)
             self.cursor = self.db_connection.cursor()
@@ -279,7 +278,6 @@
             self.tracks.append(Track(row[0], row[1], row[2], row[3], self,
                                      row[5], row[6], row[7], row[8], row[9],
                                      row[10], row[11]))
- #self.artist = row[3] #FIXME: Should we set artist somewhere else?
             self.total_length += int(row[9])
         if len(self.tracks) == 0:
             raise AlbumHasNoTracks()
@@ -287,7 +285,6 @@

     def __str__(self):
         return self.title
- #return '%s - %s' % (self.artist, self.title)

     def get_title(self):
         """
@@ -444,8 +441,6 @@
         Get album that contains this Track.
         @return: Album object
         """
- #FIXME: when Album constructor removes work-around, get_album needs to
- # remove the default False and get the cursor from the media library
         if not isinstance(self.album, Album):
             album = Album(self.album, cursor)
             self.album = album
@@ -458,9 +453,6 @@
         Get album art URL of this Track
         @return: String (or None if there is no album art for this track)
         """
- #FIXME: when Album constructor removes work-around, get_album_art_url
- # needs to remove the default False and get the cursor from the media
- # library
         self.get_album(cursor) # Need to have an album object

         if self.album.has_album_art():
@@ -596,8 +588,6 @@

         query_info = CDDB.query(disc_id)[1]

- # FIXME: query_info contains code that we could use instead of TRY
- # EXCEPT
         #See CDDB documentation for more information.
         #http://cddb-py.sourceforge.net/CDDB/README

=== modified file 'pylintrc'
--- pylintrc 2009-02-08 07:01:25 +0000
+++ pylintrc 2009-02-08 20:31:58 +0000
@@ -62,7 +62,7 @@

 # XXX: rockstar - C0103 constrains method and function names to a regex
 # XXX: rockstar - W0511 checks for XXX, TODO, and FIXME
-disable-msg=I0011,R0201,R0801,R0901,R0902,R0903,R0904,R0911,R0912,R0913,R0914,R0915,R0923,W0613,C0103,W0232,W0511,W0201,E1101,E1103,W0142
+disable-msg=I0011,R0201,R0801,R0901,R0902,R0903,R0904,R0911,R0912,R0913,R0914,R0915,R0923,W0613,C0103,W0232,W0201,E1101,E1103,W0142

 [REPORTS]
@@ -295,7 +295,7 @@
 [MISCELLANEOUS]

 # List of note tags to take in consideration, separated by a comma.
-notes=FIXME,XXX,TODO
+notes=FIXME

 # checks for similarities and duplicated code. This computation may be