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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox