Code review comment for lp:~alexlauni/unity-lens-music/musicstore

Revision history for this message
Mikkel Kamstrup Erlandsen (kamstrup) wrote :

CODE:
Compiles almost without warnings froms gcc, which is a welcome feature compared to our other Vala projects! :-D

data/musicstore-scope.service.in, src/Makefile.am:
Can we prefix the musicstore-scope-daemon with unity-* like all the lens daemons? That makes process management so much easier.

Would it make sense to do a rename 's/musicstore/u1musicstore/' and 's/MusicStore/U1MusicStore/' in filenames and classes? Is it inconceivable to we'll ever add another music store?

774 +// if (cancellable != null) {
775 +// cancellable.cancel ();
776 +// cancellable = null;
777 +// }
778 +
779 + cancellable = new Cancellable ();

Delete these lines?^^

IN PRACTICE:
It appears that the entire music lens daemon becomes unresponsive until the musicstore scope daemon has replied - which blocks until it gets an answer from the u1 search service (which seems to be somewhat unreliable). I am trying to figure out if this is a bug in libunity or because it's an issue with the musicstore/music-lens.

review: Needs Fixing

« Back to merge proposal