Comment 5 for bug 815283

Revision history for this message
Michael Vogt (mvo) wrote :

Thanks for your branch. I had a look at it and the packaging is fine now. Some comments on the code itself:

/usr/bin/indicator-cpufreq-selector
should probably move into a new dir /usr/share/indicator-cpufreq or /usr/lib/indicator-cpufreq as its not useful
to run on its own but instead is auto-started.

Its great to have a manpage :) It would be good if it would explain how to add indicator-cpufreq to the panel
(i.e. if its enough to have it installed, a small note about this would be sufficient). It does not appear on my
box but maybe thats because my system does not support it?

In bin/indicator-cpufreq I see:
"""
python_path = []
if os.path.abspath(__file__).startswith('/opt'):
    syspath = sys.path[:] # copy to avoid infinite loop in pending objects
    for path in syspath:
        opt_path = path.replace('/usr', '/opt/extras.ubuntu.com/indicator_cpufreq')
        python_path.insert(0, opt_path)
        sys.path.insert(0, opt_path)
python_path = []
if os.path.abspath(__file__).startswith('/opt'):
    syspath = sys.path[:] # copy to avoid infinite loop in pending objects
    for path in syspath:
        opt_path = path.replace('/usr', '/opt/extras.ubuntu.com/indicator_cpufreq')
        python_path.insert(0, opt_path)
        sys.path.insert(0, opt_path)
"""
That looks like its accidently duplicated?

Installing the /var/lib/polkit-1/indicator-cpufreq.pkla looks odd to me, could you please explain a bit more
why its needed? It seems like the .policy file takes already care of this as the default for admin is "unix-group:admin"
(just as its in the local policy file).

Otherwise it looks great, thanks a lot for the work on this! My comments on this should probably also be forwarded
to upstream. The only thing I would really like to know more about is the /var/lib/polkit-1/indicator-cpufreq.pkla
the otherwise I think its ready for upload.