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

[Kurt J. Bosch]

Tom Gundersen, 2011-07-09 13:24:
> 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

Rebased again as suggested. Please have a look at the new thread as this 
one is a bit chaotic meanwhile.

-- 
Kurt