Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=495902
--- Comment #17 from Paul Fox pgf@laptop.org 2009-06-06 09:33:59 EDT --- (In reply to comment #16)
I'm a little disappointed, the script could be more sophisticated IMHO, e. g. it could read max brightness from /sys/class/backlight/dcon-bl/max_brightness. Take a look at http://www.catmoran.com/olpc/#xfcebrvo
i would have thought that a script that doesn't require bash, invokes no external processes (catmoran's invokes two) would be more sophisticated, not less. :-) i understand your point about max_brightness, but the values i hard-coded in the hardware, and i see little reason to ask the kernel for the value, in that case.
olpc-brightness is being run as root because of the permissions of /sys/class/backlight/dcon-bl/brightness, right? Is there no better way? Can we use hal to give user write permission?
we could -- in fact, the daemon could simply change the permissions itself. are you concerned that users can't change the brightness themselves? or that the script runs as root?
Anyway, testing was positive, everything works as you described. So let's check the outstanding issues:
OK - MINOR: BuildRoot tag OK - MAJOR: BuildArch tag OK - MAJOR: Requires: hal added OK - MINOR: Description: line breaks are at 80 characters OK - MAJOR: RPM_OPT_FLAGS are honored OK - MAJOR: Timestamps preserved
I just realized that "BuildArch: %{ix86}" is not a good idea because the buildsys will then build for i386, i486, i586, i686 and athlon. Better use ExclusiveArch: %{ix86}
should this still use the macro in that case?
One last thing: during build I see:
- make
fatal: Not a git repository fatal: Not a git repository fatal: Not a git repository fatal: Not a git repository fatal: Not a git repository
fixed.
thanks again for your help.
I wouldn't call this a blocker, but please fix it.
olpc-kbdshim-6-2.20090605git98f5b2c.src.rpm is APPROVED
P.S.: Please cc me if you submit olpc-powerd for review.