Eric Smith <e...@trueblade.com> added the comment: A couple of points:
Didn't we decide that instead of using: openlog(ident[, logopt[, facility]]) we'd use: openlog(ident, logopt=None, facility=None) (or whatever the defaults are)? I can't find a reference, but the argument was that it's how Python signatures are written, so it's clearer if they're all written this way. You should add some comments to syslog_get_argv explaining why you're handling errors the way you are. That is, why you're swallowing exceptions and continuing. Similarly with the call to PyTuple_New(0). I also think it would be clearer if using the string "python" were inside syslog_get_argv, but that's a style thing. Should the fallback be "python", or derived from C's argv[0]? Is it possible that sys.argv[0] would be unicode? Is SEP correct, or should it really be using os.path.sep and/or os.path.altsep? This is probably a nit, but I could see it being a problem under cygwin (which I haven't tested yet). Your "if" statements shouldn't all be on one line. The single-line style with braces isn't used anywhere else in this module, and it's not in the Python code base that I could see (except for the occasional macro). The example code has some extra spaces around the equal signs. It should be: syslog.openlog(logopt=syslog.LOG_PID, facility=syslog.LOG_MAIL) Thanks for doing this! ---------- _______________________________________ Python tracker <rep...@bugs.python.org> <http://bugs.python.org/issue8451> _______________________________________ _______________________________________________ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
