On Thu, Apr 24, 2008 at 12:01:52PM -0400, Eben Eliason wrote: > I'll /try/ to keep my comments mostly to the UI and leave the code > review for others. I'll also fail at that to some extent, since I > have curiosities about bits and pieces. > > On Thu, Apr 24, 2008 at 7:33 AM, Martin Dengler > <[EMAIL PROTECTED]> wrote: > > > + return self._master.flags & gst.interfaces.MIXER_TRACK_MUTE \ > > + == gst.interfaces.MIXER_TRACK_MUTE > > If MIXER_TRACK_MUTE is a bit mask, isn't the equality check redundant?
It's not a bitmask, IIUC. So no, it's not redundant AFAICS. But yes, it'd read much more nicely without the equality check if it wasn't. > > + #sometimes we get a spurious zero from one/more channel(s) > > + #we could just pick the first channel's volume, but this converges > > + #sometimes alsa fails to set all channels' volume, so try a few > > times > > That's all fairly ugly, huh? Oh well. I know! > > + badge_name = None > ... > > + self.icon.props.badge_name = badge_name > > What's the logic for leaving this in? Hmmm...I must've missed that pylint warning. The variable goes. > > + def _mute_item_text(self): > ... > > + return _(mute_item_text) > > Defining this in a function seems to work OK here, but I think I also > want icons on the menu item, which also depends on the current state. > As such, it may actually be cleaner to have an _update function of > some kind which handles both text and image together, setting them > directly instead of returning a value. Agree -- I just didn't have any icons, so no use for such a function yet. > Let's use the dialog-ok and dialog-cancel icons for now, which will > match the current mockups for the mic and camera. We can change them > easily later if we need to. dialog-ok to go with the "Unmute" text, and -cancel to go with "Mute"? > > + mute_item_text = self._model.props.muted and 'Unmute' or 'Mute' > > This is a tricky ternary stand-in. Very clever. Is it clear enough for > others? > I'll change to the proper ternary clause as recommended by bemasc. > > + mute_item_text += '...' > > This is a bad habit that everyone seems to get into. Cut this line. Sorry! I know the rule(s) you cite, and must blame lack of sleep for leaving this in. I was cargo-culting "Disconnect..." from network/wireless.py, I think. > - Eben Martin
pgpTWPw2nOiy6.pgp
Description: PGP signature
_______________________________________________ Sugar mailing list [email protected] http://lists.laptop.org/listinfo/sugar
