Re: [PATCH 4/6] populate-volatile.sh: improve this script

[Richard Purdie <richard.purdie@linuxfoundation.org>]

On Mon, 2013-01-28 at 17:05 +0800, Qi.Chen@windriver.com wrote:
> From: Chen Qi <Qi.Chen@windriver.com>
> 
> Here's a list of improvements:
> 1) Make it run correctly at rootfs time.
> 2) Handle link config items more reasonably.
> 3) Support read-only rootfs.
> 4) Avoid data loss when removing directories.
> 
> [YOCTO #3406]
> [YOCTO #3404]
> [YOCTO #3181]
> 
> Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
> ---
>  .../initscripts-1.0/populate-volatile.sh           |  287 ++++++++++----------
>  1 file changed, 143 insertions(+), 144 deletions(-)

I'm getting a little annoyed with this patch. Saul keeps sending it to
me, I keep seeing it and rejecting it.

I will not take code changes mixed in with whitespace reformatting. I've
said this once already.

This series should have a patch like:

http://git.openembedded.org/openembedded-core/commit/?h=master-next&id=35f3369e5e6365aa06914862ef8a55e21f2fb73b

and then a follow up like:

http://git.openembedded.org/openembedded-core/commit/?h=master-next&id=ca8ca587643caa81a6c6530dfead0db6fa8c27b6

which has the actual code changes in it. I did this just so I could see
what the patch was doing.

Next problem is that there are too many different code changes in this
to figure out what is going on and properly review it. It needs to be
split up further into logical changes with a clear explanation of what
is happening. Why was the cacheclear mechanism removed? Why isn't the
background parallelisation important (I know why when I think about it
but the commit message says nothing about it)?

There is also a typo of ROOT_DRI in there.

I will merge the whitespace change, just so I don't have to see it
again. Please split up the rest of the changes into a logical set and
resubmit. meld is a good tool for work like that.

Cheers,

Richard