Code review comment for lp:~jamesodhunt/ubuntu/precise/upstart/1.4

Revision history for this message
Colin Watson (cjwatson) wrote :

TESTING.sessions doesn't seem to be in the upstream tarball, so it shows up in the packaging diff. For the next upstream release, I would suggest adding it to EXTRA_DIST in Makefile.am.

init/tests/test_job_process.c should have #include <fnmatch.h>.

Finally, a thought on log rotation: there appears to be nothing in place for logrotate to tell the init daemon that it's rotated a given log file. I appreciate that it may be tricky to set this up. Would it be worth at least using the delaycompress directive, which is documented as being useful in this kind of situation?

In general, this looks very good, thank you. Most of my comments are nits which you can address at your leisure, although it would be good to make sure they get cleaned up. The only ones that I think need to be either fixed or explained before upload are:

 * uninstall SIGCHLD properly before grantpt
 * console_type_setter return value
 * possibly the memory leak in upstart-udev-bridge

review: Needs Fixing

« Back to merge proposal