From: ChenQi <Qi.Chen@windriver.com>
To: Richard Purdie <richard.purdie@linuxfoundation.org>
Cc: Zhenfeng.Zhao@windriver.com, openembedded-core@lists.openembedded.org
Subject: Re: [PATCH 4/6] populate-volatile.sh: improve this script
Date: Mon, 18 Feb 2013 11:18:46 +0800 [thread overview]
Message-ID: <51219D96.2070907@windriver.com> (raw)
In-Reply-To: <1360161619.28450.12.camel@ted>
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
>
>
>
>
next prev parent reply other threads:[~2013-02-18 3:35 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-01-28 9:05 [PATCH 0/6] Readonly rootfs support Qi.Chen
2013-01-28 9:05 ` [PATCH 1/6] sysvinit: add ROOTFS_READ_ONLY variable to rcS-default Qi.Chen
2013-01-28 9:05 ` [PATCH 2/6] image.bbclass: add a hook funtion to support readonly rootfs Qi.Chen
2013-01-28 9:05 ` [PATCH 3/6] core-image.bbclass: support read-only rootfs Qi.Chen
2013-01-28 9:05 ` [PATCH 4/6] populate-volatile.sh: improve this script Qi.Chen
2013-02-06 14:40 ` Richard Purdie
2013-02-18 3:18 ` ChenQi [this message]
2013-01-28 9:05 ` [PATCH 5/6] initscripts: let populate-volatile.sh create the /tmp link Qi.Chen
2013-01-28 9:05 ` [PATCH 6/6] initscripts: add read-only-rootfs-hook.sh script Qi.Chen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=51219D96.2070907@windriver.com \
--to=qi.chen@windriver.com \
--cc=Zhenfeng.Zhao@windriver.com \
--cc=openembedded-core@lists.openembedded.org \
--cc=richard.purdie@linuxfoundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.