On Sat, 20 July 2013 13:35:08 +0200, Stefan Gehn wrote:
> 
> - VARIANTS could be replaced by an enum for the four variants. That way
> setup of the arrays in setup_sysfs_battery would be a bit more
> descriptive. The hardcoded indices inside the code and the VARIANTS
> define could get out of sync easily if somebody has to touch that code
> again. I thought about something along the lines of:
> 
> typedef enum SysChargeVariants_ { SCVEnergy, SCVEnergyDesign, SCVCharge,
> SCVChargeDesign, SCVCapacity, SCVCount } SysChargeVariants;

Would be fine with me, with one caveat: I really don't believe anyone
will touch that code again.  Gkrellm is effectively abandonware.  How
else could a patch like mine that - code issues aside - fixes an
easily reproducable bug linger for 15 months without comment? ;)

> - The patch drops all debug output in setup_sysfs_battery(). I think the
> output should be ported to at least log the stored paths for the
> multiple variants.

That is actually a good thing.  Debug output is random noise in the
source code and makes it harder to read/understand/change.  That
downside always exists.  In some cases debug output also has upsides
that sufficiently counter the downsides.  I didn't need the debug
output at all and the gkrellm maintainer... *cough*

Btw, how much inconvenience does it involve to become the gkrellm
maintainer for Debian?  All I can promise is to fix any bugs that
happen to annoy me personally and occasionally merge patches from
others, provided the code is reasonable.  If you can do a better job
than that, go Stefan go!  This is just a fallback offer.

Jörn

--
Linux [...] existed just for discussion between people who wanted
to show off how geeky they were.
-- Rob Enderle


-- 
To UNSUBSCRIBE, email to [email protected]
with a subject of "unsubscribe". Trouble? Contact [email protected]

Reply via email to