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

ChenQi
2013-02-18T03:35:02Z

On 02/06/2013 10:40 PM, Richard Purdie wrote:
> 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.

Thanks for your advices.
I'll rework on this patch and resubmit it.

Thanks,
Chen Qi

> Cheers,
>
> Richard
>
>
>
>