Re: [PATCH 1/1] oe-init-build-env, scripts/oe-buildenv-internal: add error detecting for $BDIR

[Darren Hart <dvhart@linux.intel.com>]

On 08/04/2011 05:07 AM, Richard Purdie wrote:
> On Tue, 2011-08-02 at 21:06 -0700, Darren Hart wrote:
>>
>> On 08/02/2011 04:43 AM, Richard Purdie wrote:
>>> On Tue, 2011-08-02 at 14:08 +0800, Dexuan Cui wrote:
>>>> [YOCTO #671]
>>>>
>>>> "readlink -f" in Ubuntu 10.04 is buggy: it doesn't ignore a trailing / (e.g.,
>>>> "readlink -f /tmp/non-existent-dir/" returns nothing, but according to
>>>> http://www.gnu.org/s/coreutils/manual/coreutils.pdf it should do that --
>>>> hence we get bug 671. It seems Ubuntu 10.10 or even later Ubuntu 11.04,
>>>> and other Linux distributions(e.g., Open Suse 11.4) haven't such an issue.
>>>>
>>>> So I think we should detect this and ask Ubuntu 10.04 users to avoid supply
>>>> a path with trailing slash here.
>>>>
>>>> Moreever, I also add the detection of non-existent path, e.g.,
>>>> source oe-init-build-env /non-existent-dir/build
>>>> can be detected and we'll print an error msg.
>>>> And, if we get errors in oe-buildenv-internal, we should stop the script
>>>> and shouldn't further run.
>>>>
>>>> Signed-off-by: Dexuan Cui <dexuan.cui@intel.com>
>>>
>>> Merged to master, thanks.
>>
>> For a patch to address a relatively benign bug I thought the standard
>> procedure would be for it to await feedback for more than 5 hours. I was
>> hoping to have an opportunity to review this fix as I was working with
>> the team in root causing the bug.
> 
> It is near impossible for me to tell who (if anyone) is working jointly
> on an issue or expecting to review a patch. All I see are the complaints
> when things don't merge promptly or something less than ideal merges too
> soon (i.e. I can't win) :(.


In this case I was trying to refer back to what I had understood to be
the norm (waiting for 24 hours) to allow for feedback. I know it wasn't
a hard rule, but I didn't see any degree of urgency with this patch. If
your process is different than my understanding, please correct my
thinking so I know what to expect going forward. If not, then the above
is just meant as a friendly reminder that I, at least, am operating
under the assumption that patches will have a 24 hour review window
unless there is a pressing need to merge them sooner.


> If a group of people want to review and ack a patch before its accepted
> can you please indicate this somewhere in the patch so I stand a chance
> of figuring out what people are expecting me to do...


This is a critical part of the patch review process. 100% agreed. The
send-pull-requuest script knows to look for the CC: line below the
Signed-off-by for people whose input is relevant to the patch. Please
make use of this function to first notify them (people who helped on the
bug, recipe authors, maintainers, bug introducer, or people who have
recently modified the files in question) that a change is on the list
that may benefit from their review and second to notify the person
responsible for merging the patch that there are others involved who
should have an opportunity to provide feedback before the patch is merged.

Thanks,

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel