Code review comment for lp:~fazerlicourice/music-app/test-empty-library

Revision history for this message
Andrew Hayzen (ahayzen) wrote :

Some more inline comments :-)

1) I don't think you need to add the visible: false as the object is created when it is required.
2) I wonder if this method should be get_library_empty_state() or get_library_empty_state_page() ? To match the pattern of the other methods?
3) Can the double space be fixed to a single space?
4) This import can be removed, see further details below.
5) If the first os.remove() fails, then this throws the exception before it can attempt to remove the others?
6) Change this to
except Exception as e:
    logger.debug("Mediscanner patching failed %s" % e)

Or for exactly the same
except Exception as e:
    logger.debug("Mediscanner patching failed %s" % type(e))

review: Needs Fixing

« Back to merge proposal