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

Thanks for taking time to review Paul,

> 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.

Yep, it seems to have no CDDB test. So I'll make one but before can you tell me which test structure is the good one because I've not followed all your modifications recently. Is it the *TestCommon* like in the music-test for instance?

> 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.

Yeah, reading the CDDB API doc file I came to the conclusion that it was handled by the CDDB Class but I did not really check. So I'll fix that with a timeout if my CDDB test result is not good.

Thanks again,

Samuel,