(I don't intend to sponsor this package.)

* Jacopo Lorenzetti <[email protected]>, 2012-05-09, 02:13:
http://mentors.debian.net/debian/pool/main/g/gnome-session-shutdown/gnome-session-shutdown_1.81-1.dsc

I'd use "=" instead of ":=" in debian/rules, so that the variable is not uselessly evaluated even when it's not used. (And it won't be used most of the time.)

The definition of DEBIAN_DIR could be simplified to:

$(dir $(firstword ${MAKEFILE_LIST}))

That said, I wouldn't bother with implementing get-orig-source target when pristine upstream tarballs is being used.

What is build-dependency on docbook2x for?

I see some bugs and weirdnesses in upstream code:

Line 46:
import string

string module is so 90s! ;) Most of functions from that module (including the ones you use) are deprecated.

Line 52:
# If running outside X try connecting to main display
try:
        display = os.environ['DISPLAY']
except KeyError:
        os.environ['DISPLAY'] = ':0.0'

This is weird. I haven't seen any program behaving that way. It's user responsibility to set DISPLAY correctly, and I wouldn't want any program to second-guess me on that matter.

Line 233:
                        wall.communicate(message)[0]

This looks a bit odd. Was the expression supposed to be assigned to something? If not, the "[0]" part is redundant.

Line 287:
                        pidf.close

This is no-op. I guess it should be: "pidf.close()".

Line 288:
                except:
                        return False

Catching all exceptions is almost always wrong (unless you re-raise them later), for two reasons:
- you don't want to catch things like KeyboardInterrupt;
- you don't want to inadvertently ignore an exception that was raised because of a bug in your program.

Please catch only exceptions you actually expect to be raised during normal program execution.

Line 304:
                        if e.errno != errno.EEXIST:

You didn't import the errno module.

Lines 313-131: similar problems like in 287-288.

Line 349:
        gettext.install('gnome-session-shutdown', LOCALE_DIR, unicode=1)

AFAIK if you can omit the LOCALE_DIR part, Python will do the right thing. No need to hard-code path to locale directory.

Line 360:
                for fname in os.listdir('/proc'):
                        if fname.isdigit():
                                if int(fname) >= 1000:

This looks very dubious to me. Why ">= 1000"?

Running pygettext on the source file results in:

$ pygettext gnome-session-shutdown
*** gnome-session-shutdown:131: Seen unexpected token "+"
*** gnome-session-shutdown:375: Seen unexpected token "+"
*** gnome-session-shutdown:397: Seen unexpected token "+"
*** gnome-session-shutdown:429: Seen unexpected token "+"

--
Jakub Wilk



--
To UNSUBSCRIBE, email to [email protected]
with a subject of "unsubscribe". Trouble? Contact [email protected]
Archive: http://lists.debian.org/[email protected]

Reply via email to