On Wed, 2010-07-07 at 14:03 +1000, Allan McRae wrote:
Here is a quick review on all these patches. I recommend that the lvm and crypttab changes get a decent amount of testing before these go live as they are the biggest changes being done.
Tighten up the console size finding code a bit. Add some white space in test construct: if ((STAT_COL==0)); then if (( STAT_COL == 0 )); then and throughout these patches
Maybe when all the other bugs are fixed. :)
Simplify the code that clears USECOLOR. The following condition is removed with no commit message to explain why if [ $? = 3 ]; then TERM_COLOURS=8
An exitval of 3 means tput has no idea what terminal type we are running on, and I figured that it is after to default to not using colorized output in that case.
Replace trivial use of grep with bash regex conditional. - if [ -n "$CONSOLEMAP" ] && echo "$LOCALE" | /bin/grep -qi utf ; then + [[ $CONSOLEMAP && $LOCALE =~ UTF|utf ]] && CONSOLEMAP=""
Use ... && ${LOCALE,,} == utf ]] to accurately replicate the grep
Hmmm... I had not seen that parameter expansion before. New to bash 4.1?
Replace slightly too long echo staement with a here document. ^^ typo I actually find the echo more readable
Well, then Thomas can keep it or drop it as he prefers.
Change the daemon runnign loop to use a case statement. Quote daemon names. Merge these commits
Both rc.single and rc.shutdown use the same code to kill everything. + # $1 = where we are being called from. + # This is used to determine which hooks to run. -> Add separater line here... + # Find daemons NOT in the DAEMONS array. Shut these down first
Why has this been removed: -if [ -x /etc/rc.local.shutdown ]; then - /etc/rc.local.shutdown -fi Ah... it has been moved to another place in another commit. Please document these sorts of changes in your commit message and preferably do the entire move in one commit.
Will fix.
Flatten LVM deactivation if block in rc.shutdown. This change does not do the same thing and I do not see where it gets replicated -if [ -f /etc/crypttab -a -n "$(/bin/grep -v ^# /etc/crypttab | /bin/grep -v ^$)" ]; then +if [[ -f /etc/crypttab ]]; then
All the second bit of that test does is to see if there is actual content in /etc/crypttab. I handle that in the read loop by zapping comments and blank lines with parameter expansion and conditional checks instead -- the greps end up reading the whole file anyways. Replicated via parameter expansion and conditionals: [[ $line && ${line:0:1} != '#' ]] || continue eval nspo=("${line%#*}")
Also another: +if [[ $USELVM =~ yes|YES -> ${USELVM,,} == yes
bashify bringing up the loopback adaptor. Add a commit message as that is doing a lot more than bashifing.
Already fixed in the last round of rebasing.
Bashify locale setting. $LOCALE =~ utf|UTF
Allan
-- Victor Lowther LPIC2 UCP RHCE