[arch-projects] [initscripts] [PATCH 00/13] A new rag rug roll (rebased)

[Tom Gundersen]

Hi Kurt,

I'm just reviewing this. Before I get started, could I ask you to add
a few more details to your commit messages? Many things that look
obvious to us now might not be obvious to future contributors, so I
would prefer too many rather than to few comments.

What I have in mind:

On Sun, Jul 3, 2011 at 10:31 PM, Kurt J. Bosch
<kjb-temp-2009 at alpenjodel.de> wrote:
> Kurt J. Bosch (13):
>  functions/rc.single: Clean up whitespace

That's ok.

>  rc.sysinit: Fix stat_busy block indentation

A sentence explaining the "policy" would be nice: "Align
stat_{fail,done} with stat_busy".

>  functions: Get rid of superfluous braces in udevd_modprobe()

Why are they superfluous? This has been pointed out on the list, and
it is pretty basic, but I prefer if these kind of things are pointed
out explicitly.

>  functions: Add missing quotes in mount_all()

Why are they needed? It is nice to know the difference between "this
is the right way to do it, even if it does not fix a bug" and a bug
fix.

>  functions/rc.multi: Strip paths from binaries
>  rc.sysinit/functions: Refactor kill_everything/fsck_all/mount_all

Those are good.

>  rc.sysinit: Declare $FORCEFSCK read-only to prevent custom hooks from
>    messing

Fine I guess, but it would be better if you split the summary and
justification in two lines:

Declare $FORCEFSCK read-only

This prevents <name of hooks> from overwriting the value.

>  functions/rc.sysinit: Refactor fsck-redirection to prevent breakage

Good comments. Question: Is it necessary to declare the empty variables?

>  functions: Sanitize status() behaviour to make it more intuitive

Good comments.
I agree with returning the error code. Not sure about the redirection.
If you have a usecase (not the one below), then maybe.

>  rc.sysinit: Simplify setting hostname by using status() even with
>    redirection now

I don't think this works. Before this patch the status encapsulates
the writing to /proc, after this patch it only encapsulates the echo
command, so in case we cannot write to /proc the status message does
not behave as intended.

>  functions: Deprecate in_array(), Add replacement is_in_array()

I agree with changing to use ck_daemon, probably about the deprecation
too, not sure about adding replacement function without a user though.

>  functions: Add stop_all_daemons() and related *_prestopdaemons hook
>  functions: Speed up reboot/shutdown by recognizing killall5 exit code
>    2

Good comments, I'll have to look into  the killall5 stuff before
deciding though.


Cheers,

Tom