[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Automated Kerberos installation ready

Am Samstag, den 17. Juli hub Jonas Smedegaard folgendes in die Tasten:


You are right will all of your critics :-)
I know that this packages is kind of hack-o-rama.
It was ment as proof-of-concept and was written between 21:00 and 9:00 o
clock, so please don't be too hard.
If I would make a stable implementation, I'l have a look at many many
things, that could be done better!

> Sorry - your package is too specific for me for anything but 
> inspiration: Skolelinux default are hardcoded all over (instead of the 
> relatively few Skolelinux-specific values isolated in a master conffile 
> and applied using m4 or similar).

Good idea.

> I do have some more general suggestions/comments on your coding style, 
> however. Use whatever you can - please don't get offended if my 
> assumptions are wrong in some of it (they quite likely are):

>  * Use "cp -p" instead of just "cp" when backing up old files


>  * Write both copyright and licensing info (a cow is _not_ enough ;-) )


>  * Use "set -e" in shell scripts to fail on errors

Might be a good idea.

>  * Avoid piping to /dev/null - it might be crucial warnings you hide
>    - instead (for bin/krbuser) implement --silent option

That is a good idea.
In general my scripts have something like that.
That was just to have it work.

>  * Avoid hardcoding -x when debugging - instead use "sh -x script.sh"

That is a left over debugging option, I just forgot to delete that.

>  * In perl use 'system "echo" $arg' instead of 'system "echo $arg"'

I don't know perl, that was only proof of concept. :)
I know Andreas or others would find a better way.
Again: only proof of concept

>  * Why hide common functions? Just don't make it executable

OK, that's desgin opinions.

>  * Use mktemp instead of a hardcoded static secret tempdir

Didn't know mktemp, I will look into it.

>  * Use /bin/sh instead of /bin/bash if at all possible


>  * postinst must be idempotent: Try configure twice...

That is a know bug, I were to tired to fix that.

>  * Remove irrelevant parts of debian/rules


>  * Seems you have old code in debian-edu-krb5-0.1 subfolder

That is possible :-)

Again, my idea was to show that it is posibble, to do that automatically.
I do not want to win a codingstyle-record with that, I know that it
is a bad hack.

Please *don't* think that I in general write such style. :)

	Follow the white penguin.

Reply to: