Code review comment for lp:~stumbaumr/ltsp/ltsp

Revision history for this message
Vagrant Cascadian (vagrantc) wrote :

Thanks for the patches!

The KIOSK_OPTIONS part in kioskSession is not be needed, it's handled by passing the environment variable in screen.d/kiosk to kioskSession in recent LTSP versions.

Regarding the logging, it would be better to have all the various logging functions in a function, which could include a conditional to enable/disable it. The conditional might be best as a bootprompt option, given that some of the logging happens before lts.conf is processed.

There are significant portions of the ltsp codebase which run in "set -e", and so unconditionally calling "logger" or writing to /dev/kmsg could actually result in unhandled errors, though a brief review of this particular code was not running under "set -e". Handling this in a function would also be cleaner.

review: Needs Fixing

« Back to merge proposal