[RFC] review process

[RFC] review process

[Frans Meulenbroeks]

It is a good rule that important changes (toolchain, infrastructure)
are reviewed before being committed.
This is e.g. also specified in
http://wiki.openembedded.net/index.php/Commit_Policy

However, recently I've seen some issues that our review process does
not seem to work or is abused.

I see two things happening.
- patches are submitted for review but do not gain any feedback in a
reasonable time. I have several patches in the queue that did not get
any feedback.
- people are abusing their powers by rejecting changes without
motivation. See e.g [1] and [2]. I feel if you reject a patch you have
an obligation to explain why you rejected it.

Seems our review process is flawed.
I propose to introduce the following rules.

1. If a patch does not get any review feedback in X weeks time; it is
ok to apply it. People who need more time to review a patch can
mention that in a short reply. In that case they are granted Y more
weeks to review.
(suggestion: X = Y = 2)
2. If someone NAKs a patch it is obligatory to provide an explanation
why the patch is not good.
Rationale is that
a) people can fix the problem seen by the reviewer
b) people learn from it
c) if there is a disagreement it can be discussed (and if needed
raised to the TSC)
3) NAKs that are not motivated/explained can be ignored as not given.

Your feedback, suggestions, additions, amendments, whatever is appreciated.

Frans

[1] http://lists.linuxtogo.org/pipermail/openembedded-devel/2010-August/023374.html
[2] http://lists.linuxtogo.org/pipermail/openembedded-devel/2010-August/023270.html

Re: [RFC] review process

[Graeme Gregory]

 On 27/08/10 13:15, Frans Meulenbroeks wrote:
> 1. If a patch does not get any review feedback in X weeks time; it is
> ok to apply it. People who need more time to review a patch can
> mention that in a short reply. In that case they are granted Y more
> weeks to review.
> (suggestion: X = Y = 2)
I have always been a supported of apply it and see who screams. I have
suggested before that after 2 weeks without review any non core patch is
obviously OK.

G

Re: [RFC] review process

[Gary Thomas]

On 08/27/2010 06:24 AM, Graeme Gregory wrote:
>   On 27/08/10 13:15, Frans Meulenbroeks wrote:
>> 1. If a patch does not get any review feedback in X weeks time; it is
>> ok to apply it. People who need more time to review a patch can
>> mention that in a short reply. In that case they are granted Y more
>> weeks to review.
>> (suggestion: X = Y = 2)
> I have always been a supported of apply it and see who screams. I have
> suggested before that after 2 weeks without review any non core patch is
> obviously OK.

That said, what good does it do for those affected to scream if
the originator simply ignores it??

-- 
------------------------------------------------------------
Gary Thomas                 |  Consulting for the
MLB Associates              |    Embedded world
------------------------------------------------------------

Re: [RFC] review process

[Frans Meulenbroeks]

2010/8/27 Gary Thomas <gary@mlbassoc.com>:
> On 08/27/2010 06:24 AM, Graeme Gregory wrote:
>>
>>  On 27/08/10 13:15, Frans Meulenbroeks wrote:
>>>
>>> 1. If a patch does not get any review feedback in X weeks time; it is
>>> ok to apply it. People who need more time to review a patch can
>>> mention that in a short reply. In that case they are granted Y more
>>> weeks to review.
>>> (suggestion: X = Y = 2)
>>
>> I have always been a supported of apply it and see who screams. I have
>> suggested before that after 2 weeks without review any non core patch is
>> obviously OK.
>
> That said, what good does it do for those affected to scream if
> the originator simply ignores it??

Raise it. And if it cannot be resolved or discussed probably raise it
with the TSC.
Do you have a specific problem? If so I suggest opening a different
mail thread about it. Let's keep this one clean for disucssing the
process.

Frans

Re: [RFC] review process

[Dr. Michael Lauer]

Am Freitag, den 27.08.2010, 14:15 +0200 schrieb Frans Meulenbroeks:
> 1. If a patch does not get any review feedback in X weeks time; it is
> ok to apply it. People who need more time to review a patch can
> mention that in a short reply. In that case they are granted Y more
> weeks to review.
> (suggestion: X = Y = 2)
> 2. If someone NAKs a patch it is obligatory to provide an explanation
> why the patch is not good.
> Rationale is that
> a) people can fix the problem seen by the reviewer
> b) people learn from it
> c) if there is a disagreement it can be discussed (and if needed
> raised to the TSC)
> 3) NAKs that are not motivated/explained can be ignored as not given.
> 
> Your feedback, suggestions, additions, amendments, whatever is appreciated.

I agree with that. If someone does the work to create a patch he thinks
is worthwhile to apply the least we can do when NAKing is to explain why
it is not a good idea.

:M:

Execution of Build Output Failing (Testing)

[openembedded]

Hi,
As I reported recently, I am having issues getting my OE output up and running on my h1940 device. It seems to be some sort of issue between the kernel and initrd files, so to debug this I figured that I'd build and test on QEMU (hoping that the recent udev related issues were my problem, and to make sure that I could build "properly").
Given this, I went ahead and built console-image (angstrom 2008.1 distro) for qemuarm, to see if it was perhaps just me having issues (which is still possible). However, I took the output from the qemuarm build and tried to run it ... and still have issues (not the same as my h1940, but still fatal).
On QEMU (verified to work with the QEMU provide example ARM images) I am able to boot, at least part way, but end up with a FATAL message about a missing directory ... /lib/modules/2.6.17-rc3.
Is this a known issue? Any proposed fixes or workarounds for this?
Thanks!
... Russell

Re: [RFC] review process

[Roman I Khimov]

[-- Attachment #1: Type: Text/Plain, Size: 983 bytes --]

В сообщении от Пятница 27 августа 2010 16:15:01 автор Frans Meulenbroeks 
написал:
> 1. If a patch does not get any review feedback in X weeks time; it is
> ok to apply it. People who need more time to review a patch can
> mention that in a short reply. In that case they are granted Y more
> weeks to review.
> (suggestion: X = Y = 2)
> 2. If someone NAKs a patch it is obligatory to provide an explanation
> why the patch is not good.
> Rationale is that
> a) people can fix the problem seen by the reviewer
> b) people learn from it
> c) if there is a disagreement it can be discussed (and if needed
> raised to the TSC)
> 3) NAKs that are not motivated/explained can be ignored as not given.

Looks good to me, especially the first one as some defined period of time to 
wait is useful. Two weeks is OK.

-- 
 http://roman.khimov.ru
mailto: roman@khimov.ru
gpg --keyserver hkp://subkeys.pgp.net --recv-keys 0xE5E055C3

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 205 bytes --]

Re: Execution of Build Output Failing (Testing)

[Khem Raj]

On Fri, Aug 27, 2010 at 6:46 AM,  <openembedded@rkmorris.us> wrote:
> Hi,
> As I reported recently, I am having issues getting my OE output up and running on my h1940 device. It seems to be some sort of issue between the kernel and initrd files, so to debug this I figured that I'd build and test on QEMU (hoping that the recent udev related issues were my problem, and to make sure that I could build "properly").
> Given this, I went ahead and built console-image (angstrom 2008.1 distro) for qemuarm, to see if it was perhaps just me having issues (which is still possible). However, I took the output from the qemuarm build and tried to run it ... and still have issues (not the same as my h1940, but still fatal).
> On QEMU (verified to work with the QEMU provide example ARM images) I am able to boot, at least part way, but end up with a FATAL message about a missing directory ... /lib/modules/2.6.17-rc3.
> Is this a known issue? Any proposed fixes or workarounds for this?

oe.dev works well on qemuarm here with

PREFERRED_VERSION_usbutils_local                = "0.86"
PREFERRED_VERSION_udev_local                    = "151"

in local.conf and minimal/angstrom distro last time I tried it booted
fine. you can also adapt
contrib/qemu/run-qemu.sh to run OE in qemu.

> Thanks!
> ... Russell
> _______________________________________________
> Openembedded-devel mailing list
> Openembedded-devel@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-devel
>

Re: Execution of Build Output Failing (Testing)

[openembedded]

Hi,
Thanks for the quick response!
What's strange is that watching the boot in QEMU it seems to be looking for 2.6.17-rc3 inside the /lib/modules directory ... but if I mount my initrd (un-gzip, then mount as ext2), and look inside this directory - the directory is 2.6.34 instead! Somewhere / somehow there seems to be a versioning issue perhaps?
Thoughts?
Thanks again,... Russell


On Fri, Aug 27, 2010 12:41  PM, Khem Raj <raj.khem@gmail.com> wrote:
> On Fri, Aug 27, 2010 at 6:46 AM,  <openembedded@rkmorris.us> wrote:
> > Hi,
> > As I reported recently, I am having issues getting my OE output up and running on my h1940 device. It seems to be some sort of issue between the kernel and initrd files, so to debug this I figured that I'd build and test on QEMU (hoping that the recent udev related issues were my problem, and to make sure that I could build "properly").
> > Given this, I went ahead and built console-image (angstrom 2008.1 distro) for qemuarm, to see if it was perhaps just me having issues (which is still possible). However, I took the output from the qemuarm build and tried to run it ... and still have issues (not the same as my h1940, but still fatal).
> > On QEMU (verified to work with the QEMU provide example ARM images) I am able to boot, at least part way, but end up with a FATAL message about a missing directory ... /lib/modules/2.6.17-rc3.
> > Is this a known issue? Any proposed fixes or workarounds for this?
> 
> oe.dev works well on qemuarm here with
> 
> PREFERRED_VERSION_usbutils_local                = "0.86"
> PREFERRED_VERSION_udev_local                    = "151"
> 
> in local.conf and minimal/angstrom distro last time I tried it booted
> fine. you can also adapt
> contrib/qemu/run-qemu.sh to run OE in qemu.
> 
> > Thanks!
> > ... Russell
> > _______________________________________________
> > Openembedded-devel mailing list
> > Openembedded-devel@lists.openembedded.org
> > http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-devel
> >
> 
> _______________________________________________
> Openembedded-devel mailing list
> Openembedded-devel@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-devel
>
From raj.khem@gmail.com Fri Aug 27 22:17:59 2010
Received: from mail-iw0-f175.google.com ([209.85.214.175])
	by linuxtogo.org with esmtp (Exim 4.69)
	(envelope-from <raj.khem@gmail.com>) id 1Op5MY-000859-3Y
	for openembedded-devel@lists.openembedded.org;
	Fri, 27 Aug 2010 22:17:50 +0200
Received: by iwn2 with SMTP id 2so2795409iwn.6
	for <openembedded-devel@lists.openembedded.org>;
	Fri, 27 Aug 2010 13:16:54 -0700 (PDT)
DKIM-Signature: v

Re: [RFC] review process

[Koen Kooi]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 27-08-10 14:15, Frans Meulenbroeks wrote:
> It is a good rule that important changes (toolchain, infrastructure)
> are reviewed before being committed.
> This is e.g. also specified in
> http://wiki.openembedded.net/index.php/Commit_Policy
> 
> However, recently I've seen some issues that our review process does
> not seem to work or is abused.
> 
> I see two things happening.
> - patches are submitted for review but do not gain any feedback in a
> reasonable time. I have several patches in the queue that did not get
> any feedback.
> - people are abusing their powers by rejecting changes without
> motivation. See e.g [1] and [2]. I feel if you reject a patch you have
> an obligation to explain why you rejected it.

If you want to delete a recipe I maintain, the burden is on you or are
you saying that maintainers aren't in charge of their recipes anymore?
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.5 (Darwin)

iD8DBQFMeOQmMkyGM64RGpERAu7PAKCgv9CNgRClgz28jyNeHqANqgBsdwCgr4Lp
g5tptYYjCTFtUZnOukdJ3Kg=
=wUcg
-----END PGP SIGNATURE-----

Re: [RFC] review process

[Frans Meulenbroeks]

2010/8/28 Koen Kooi <k.kooi@student.utwente.nl>:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 27-08-10 14:15, Frans Meulenbroeks wrote:
>> It is a good rule that important changes (toolchain, infrastructure)
>> are reviewed before being committed.
>> This is e.g. also specified in
>> http://wiki.openembedded.net/index.php/Commit_Policy
>>
>> However, recently I've seen some issues that our review process does
>> not seem to work or is abused.
>>
>> I see two things happening.
>> - patches are submitted for review but do not gain any feedback in a
>> reasonable time. I have several patches in the queue that did not get
>> any feedback.
>> - people are abusing their powers by rejecting changes without
>> motivation. See e.g [1] and [2]. I feel if you reject a patch you have
>> an obligation to explain why you rejected it.
>
> If you want to delete a recipe I maintain, the burden is on you or are
> you saying that maintainers aren't in charge of their recipes anymore?

One of the reasons that someone submits a patch is *because* he is not
the maintainer of the recipe.
Like Mickey says it is not more than polite to tell the submitter (who
invested time in creating the patch) why you reject the patch.
Maintainership does not only come with rights but also with
responsibilities. There is no reason to be rude.
Note also that OE is a team effort. One is the maintainer of the
recipe, not the dictator of the recipe.

Also I (and I think many with me) would appreciate it if you showed
the same respect to the recipes of others as you expect others to show
to your recipes.

Frans.

PS: Wrt the maintainership of the recipes: I do not want to divert
this topic into a discussion on that, but will react to that in the
bitbake world thread started by Khem.

Re: [RFC] review process

[Frans Meulenbroeks]

As there have not been any more replies in this thread in more than a
week, I feel the discussion has ended.

May I ask the TSC to conclude on this and provide a recommended way of working?

Best regards, Frans