← Back to team overview

ayatana-commits team mailing list archive

Re: [Merge] lp:~cjcurran/indicator-sound/slider_length_fixing into lp:indicator-sound

 

Review: Needs Information
Some questions

1. I don't see where the new ido signals are connected and handled: it seems everything was removed or commented out. Is that managed in another part of the code?

2. if device_available == FALSE, shouldn't you set the state to some reasonable default anyway? so that if the device availability fluctuates, you do not rely on the last state when it was available?

3. What happens if the state changes while an animation happens? is the icon at the end of the animation going to reflect the new state, or the old one, or just the end of the animation?

4. in catch_signal_sink_mute_update() in the if/else block it's not clear what happens: i'd suggest putting pre-conditions at the entry of the function with some g_exit_if_fail (device_available != TRUE) or something similar

5. in catch_signal_sink_availability_update(), shouldn't you call update_state whatever the new value is?
-- 
https://code.launchpad.net/~cjcurran/indicator-sound/slider_length_fixing/+merge/21132
Your team ayatana-commits is subscribed to branch lp:indicator-sound.



Follow ups

References