Erwann Chenede wrote:
> Hi All,
>
> This fix adds 2 new scripts to the SUNWcompiz package. See
> http://www.opensolaris.org/jive/thread.jspa?messageID=209065𳂩
> for the architecture (ARC) details.
> One script (compiz-configure) is a pyhton GUI that checks the
> system capabilities wrt to running compiz. The second one is a perl script
> that modify the Xserver configuration to run compiz on the system.
>
> The source can be found here :
> http://www.gnome.org/~erwannc/compiz/gnome-integration-0.6.2.v1.tar.bz2
I've only looked at the modify-xorg-conf script in there, and I've got some
questions.
- Unless I'm misreading the {} nesting, you're calling
/usr/bin/nvidia-xconfig in configure_xorg_conf_intel - is that
really the right thing to do?
- Checking prtconf for the string "nvidia" seems like you risk hitting
false positives when other nvidia devices are present, like nvidia
ethernet chips. I'd check /usr/X11/bin/constype output for NVDAnvda.
(That's how the OpenGL library version selector & the Xorg server
determine if the nvidia driver is in use.)
- The hardcoded list of pci id's for Intel & ATI chipsets also seems like
it will be a challenge to keep accurate and up-to-date.
- While not required, bonus points if your perl script follows the guidelines
in http://muskoka.eng/~sch/perl/style.html (I don't know if those are
published externally yet) - at the least, expect a bug report filed by
Install QA on any package that includes a perl script and doesn't have
perl listed in the package dependencies or doesn't use /usr/perl5/bin/perl
instead of /usr/bin/perl to ensure it uses the OS-bundled perl instead of
another version the sysadmin changed the /usr/bin/perl link to point to.
- Stylistically, there's a lot more `` commands than I'd use in a perl script
`date` could be replaced by POSIX::strftime, `mv` by rename(), etc.
--
-Alan Coopersmith- alan.coopersmith at sun.com
Sun Microsystems, Inc. - X Window System Engineering
