Review: Needs Fixing

* all tests fail for me. it looks like the pattern for XDG_RUNTIME_DIR is 
harcoded to /run/user/NAME, but on my saucy machine it's /run/user/UID instead. 
I'd write something like:
-import pwd
-SESSION_DIR_FMT = '/run/user/%s/upstart/sessions'
+SESSION_DIR_FMT = 'upstart/sessions'
-            user = pwd.getpwuid(os.geteuid())[0]
-            watch_manager.add_watch(SESSION_DIR_FMT % user, mask)
+            session = os.path.join(os.environ['XDG_RUNTIME_DIR'], 
SESSION_DIR_FMT)
+            watch_manager.add_watch(session, mask)

to stay compatible with upstart, as that's how the sessions path is determined.

* currently "python-upstart.py" is installed into $prefix/bin/python-upstart 
which is incorrect. I looks like you want to use this file as a module, and be 
able to "import upstart" from python scripts. For this to happen it should be 
installed into $prefix/lib/python3/dist-packages/ with a name which doesn't 
have '-', e.g. upstart.py or pyupstart.py or anything you like. [*]

* would be nice to have, for make check to execute this module to run the tests
* there is now a changelog conflict.

[*] a-b is a subtract b in python, a_b is a valid variable/module name
-- 
https://code.launchpad.net/~jamesodhunt/upstart/python-upstart-module/+merge/157549
Your team Upstart Reviewers is subscribed to branch lp:upstart.

-- 
upstart-devel mailing list
[email protected]
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/upstart-devel

Reply via email to