Code review comment for lp://staging/~gary-lasker/software-center/offline-opt-in-lp965397-for-quantal

I'll bring in the comments from the first MP for 5.2 (https://code.launchpad.net/~gary-lasker/software-center/offline-opt-in-lp965397/+merge/116780), that one has been superceded by this one:

Michael Vogt (mvo) wrote 9 hours ago:

On Wed, Jul 25, 2012 at 11:13:20PM -0000, Gary Lasker wrote:
> This branch implements the offline recommendations opt-in behavior as specified in bug 965397 (and the corresponding section of the referenced spec). Note that all of the combinations of turning on and off recommendations with network available and not available should do the right thing. Note also that state of the recommendations menu item is consistently updated as well.
>
> Unit tests are included.

Thanks for the branch. This looks good.

Could you please target trunk? We want to upload it there first as its
a string change so we can not land this in 5.2 as it is, it would
require coordination with the translators again. Plus when this hits
quantal we will get the translations for free so puting the fix
into 5.2 later will be easier.

I noticed that its using a static check if the network is available or
not but does not react to network state signal changes. I think that
would be really nice from a UI point-of-view. Right now if the user
has no network and opts in he sees the message that recommendations will
appear when he is online next. When he/she goes online then the
message stays there which looks a little bit confusing. It works fine
when I close/restart software-center. Or is there a mechanism for
reacting to network changes that I overlooked maybe?

> Oh, also please note that I re-enabled two of the recommendations unit tests that had been turned off. For me, these work fine now, so I'm not sure why they were disabled initially, and I wonder whether the condition that necessited that is now resolved. mvo, please check these to make sure these work for you as well.

Interessting, that works for me now as well, might be some sort of
race that is not always triggered. I will leave them enabled :)

Cheers,
 Michael