Code review comment for lp:~samuel-buffet/entertainer/bug_310413

Revision history for this message
Paul Hummer (rockstar) wrote :

Samuel-

  Great work, code looks good. I have some concerns though that will prevent
this branch from landing. First of all, this code isn't tested anywhere.
Matt and I have been REALLY strict recently about making sure that all code is
properly tested before it lands.

  The other thing I'd like you to consider is where this code is used, and how
it handles the case where there is no internet connection. Soon we're going
to have to go on a code fixing spree, handling all the issues where
Entertainer goes out to the net to fetch a resource, and how Entertainer
handles the fact that a network connection might not be available. I
suggest that if this code is accessed by the frontend, we use a gobject
timeout and have "No internet connection available" displayed instead of the
CDDB query. This needs to be fixed before this code can land as well.

 vote needs_fixing

Cheers,

--
Paul Hummer
http://theironlion.net
1024/862FF08F C921 E962 58F8 5547 6723 0E8C 1C4D 8AC5 862F F08F

review: Needs Fixing

« Back to merge proposal