Code review comment for lp:~kurt.smolderen/beat-box/beat-box

Revision history for this message
Scott Ringwelski (sgringwe) wrote :

Hey Kurt, it's easier to just use merges rather than lots of patches.

Thanks for the work! Some things for you to work on further:

1. What if user clicks close while music is playing but DOESN'T have soundmenu to reshow it? It would be really bad for the window to be "lost" meaning it is running but not accessible and has to be killed.
2. What kind of performance regression is had by checking for sound menu or unity at startup? Also, I'm not positive, but I'm pretty sure those checks are dead code meaning HAVE_INDICATE is set during compile time anyways...
3. Your rev 623 change for the raised signal should also be implemented in MPRIS.vala, which is the fallback file for when a user does not have unity but still has MPRIS services (UnityIntegration.vala uses libunity, and libunity is just a higher level api for MPRIS).
4. What if the user is in the middle of importing their music library (or other similar operations) and clicks close?

« Back to merge proposal