Review of "Alsa Mixer" version 13

Details Page Preview

Control Alsa master volume from status menu. Requires 'amixer', provided by alsa-utils package.

Extension Homepage
https://github.com/tghosgor/gnome-shell-extension-alsamixer

No comments.

Diff Against

Files

Note: Binary files aren't shown on the web site. To see all files, please download the extension zipfile.

All Versions

Previous Reviews on this Version

tghosgor posted a review
Seems stable. Previous improvements + code re-use improvements + commentary work.
fmuellner waiting for author
You are leaking file descriptors (in amixerReadCb() after having found a match), no? Minor comments: - I said we used a 'fooId' convention for variables that refer to GLib source IDs (for instance return values of .connect(), .timeout_add() or .idle_add()), not for every single global variable - you can of course name your variables whatever you like, but it makes for a very confusing read ... - you should return true from amixerUpdate() instead of creating another timeout source (note that I'm saying amixerUpdate(), not the callback passed into readVolume())
tghosgor posted a review
Yes you are right about the leak. Also fixed the timeout usage. I didn't know how to use it properly apparently.
tghosgor rejected