doherty / app-sysadmin-log-simple Goto Github PK

... or accept a hint as to what was probably meant (don't know if that's possible)

For legacy reasons, we should zero-pad single-digit numbers in filenames.

This would allow users to edit their input as they type more easily

It might be nice to echo the log entry elsewhere - IRC, Twitter, whatever. We should add a way to do that. Probably, we'd just accept the name of a binary, and call it with system("/path/to/your/binary", $text);

Right now, you'll probably install this as root, so your logdir will be owned by root, and unwritable by anyone else.

We should probably make it chgrp -R admin $logdir && chmod -R g+rw $logdir.

Logic error in the script, not the application, I think.

On some of my smoker systems I see the following failure:

#   Failed test 'No tests run for subtest "log"'
#   at t/03-log.t line 27.
Can't chown('1001', '1001', '/var/tmp/cpansmoker-1023/2017010918/ZTfqUu4aQZ/2011/2/20.log'): Operation not permitted at /tmpfs/.cpan-build-cpansand/2017010918/App-Sysadmin-Log-Simple-0.009-7/blib/lib/App/Sysadmin/Log/Simple/File.pm line 84
# Looks like your test exited with 255 just after 2.
t/03-log.t ...... 
Dubious, test returned 255 (wstat 65280, 0xff00)
Failed 3/4 subtests 
...

The tests are running as an unpriviled user, and sudo was used to switch to this user (sudo su $OTHER_USER), so SUDO_USER is set here. Of course the unprivileged user has no rights to run chown.

...which makes no sense

I just saw GumbyPAN announce a new upload of this, and immediately I was a bit baffled by your choice of name. 'App::Sysadmin' doesn't really communicate anything. It's rather generic and has little meaning on its own. Perhaps drop the Sysadmin bit?

Additionally, placing a program called 'log' in bin/ is not the best idea. You may clobber an existing system tool (thankfully you didn't choose logger which definitely does exist). I know you're going for short and concise, but you must also take care not to collide with existing system utilities. If you'd like, we can talk about naming stuff a bit at YAPC.

We don't need all of perl5i - try to pare down.

Unless we're testing the UDP functionality, we shouldn't sent UDP datagrams.

Currently we just die:

$ log --view
Can't utf8_open($fh, '<', '/home/mike/sysadmin-log/2011/2/8.log'): No such file or directory at /usr/local/share/perl/5.10.1/App/Sysadmin/Log/Simple.pm line 45

Might be time to re-think how we do external notification too... a proper plugin architecture might be worth having.