Time to take away RTBC from every Joe
First of all, yes, I broke sometimes core by jumping the gun on RTBC. But, I am aware of it. Now, let's see some other developer, quoted from IRC after asking why RTBC'd a patch without any testing or reviewing whatsoever (patch, of course, was absolutely broken and took me hours to hunt down afterwards why stuff was broken and how it's related to this one): --- It means the idea of this patch is acceptable. I guess Dries understands the risk before he commits this patch, and I am not the only one to agree about its commit. so why just give me this credit because I am the one who take RTBC action? --- Onwards, I would like to see a small handful people (me not necessarily included!) to be able to RTBC core patches. As we go on, more and more people will misunderstand what this means. A nice pyramid, a few people on top deciding the patches, a few more culling the flood of patches for them. As there is already a permission for this, a little hacking (which I am willing to do) surely would make it possible to have it just for project 3060. Kind regards, Karoly Negyesi
So let's see some negatives: *) you are closing down the Drupal review process! Elitism! Monopoly! -- I do not see why everyone needs to be able to decide a patch is ready. Review , sure , but RTBC? *) Who will decide who gets this permission? -- The core commiters. That's quite logical, they name the people whose reviews are high quality thus looks like they are worthy of this. Chain of responsibility, Dries delegates -- but the ultimate decision is in his hands. Commiters delegate further... I see this working. Regards, NK
Karoly Negyesi wrote:
Onwards, I would like to see a small handful people (me not necessarily included!) to be able to RTBC core patches. As we go on, more and more people will misunderstand what this means. A nice pyramid, a few people on top deciding the patches, a few more culling the flood of patches for them. As there is already a permission for this, a little hacking (which I am willing to do) surely would make it possible to have it just for project 3060.
No thanks. My guess is that this small handful of people who'd have this permission are the same small handful of people who are already doing a bazillion other things in core, and suddenly moving issues to RTBC would become their full-time jobs (or at least part-time). Further, instead of these "problem" users changing the status themselves, they now start PMing and e-mailing this small handful of people "RTBC my patch!!" which I can tell you right now is going to cause even further grating of nerves. :P A better approach? Education. Document the best practices, *teach* problem users what the proper usage of "RTBC" is, and they'll teach others, and so on and so on. -Angie
+1 for what Angela said. I also see we might need to document somewhere that marking a patch as RTBC is a responsibility towards that patch in terms of testing, reviewing, etc... We can't have rules for everything, sometimes pointing out problems and mistakes is a better approach. On Thu, Feb 21, 2008 at 9:19 AM, Angela Byron <drupal-devel@webchick.net> wrote:
Karoly Negyesi wrote:
Onwards, I would like to see a small handful people (me not necessarily included!) to be able to RTBC core patches. As we go on, more and more people will misunderstand what this means. A nice pyramid, a few people on top deciding the patches, a few more culling the flood of patches for them. As there is already a permission for this, a little hacking (which I am willing to do) surely would make it possible to have it just for project 3060.
No thanks.
My guess is that this small handful of people who'd have this permission are the same small handful of people who are already doing a bazillion other things in core, and suddenly moving issues to RTBC would become their full-time jobs (or at least part-time).
Further, instead of these "problem" users changing the status themselves, they now start PMing and e-mailing this small handful of people "RTBC my patch!!" which I can tell you right now is going to cause even further grating of nerves. :P
A better approach? Education. Document the best practices, *teach* problem users what the proper usage of "RTBC" is, and they'll teach others, and so on and so on.
-Angie
@see "Rename issue status "RTBC" http://drupal.org/node/156637
FYI, what Karoly proposes is exactly how the Mozilla project works. Each patch needs a "super review" from a fixed (but large) list of reveiwers. See http://www.mozilla.org/hacking/life-cycle.html I think at some point Drupal needs this. Perhaps not yet.
On 21-Feb-08, at 7:57 AM, Moshe Weitzman wrote:
FYI, what Karoly proposes is exactly how the Mozilla project works. Each patch needs a "super review" from a fixed (but large) list of reveiwers. See http://www.mozilla.org/hacking/
Those who have bothered to stick around and listen long enough while I ramble will know that I'm actually in favor of this kind of approach. I think it has several benefits : * Clear responsibility - in case of questions, complications, etc - the "buck stops" at a defined point for resolution. * Allows for "domain experts" - very frequently patches come in that can have tricky, subtle implications for some of the more complicated systems (menu, formapi, search etc etc etc) and it's *really* hard (frankly too much to ask) for the few core committers to have comprehensive understanding of all of Drupal at this level. * Makes MAINTAINERS.txt actually mean something. * Rewards solid effort from community members. etc... I think at the very least it's a concept worth pursuing. -- James Walker :: http://walkah.net/ :: xmpp:walkah@walkah.net
This is similar to what I proposed in Amsterdam as well. +1 James Walker wrote:
On 21-Feb-08, at 7:57 AM, Moshe Weitzman wrote:
FYI, what Karoly proposes is exactly how the Mozilla project works. Each patch needs a "super review" from a fixed (but large) list of reveiwers. See http://www.mozilla.org/hacking/
Those who have bothered to stick around and listen long enough while I ramble will know that I'm actually in favor of this kind of approach. I think it has several benefits :
* Clear responsibility - in case of questions, complications, etc - the "buck stops" at a defined point for resolution.
* Allows for "domain experts" - very frequently patches come in that can have tricky, subtle implications for some of the more complicated systems (menu, formapi, search etc etc etc) and it's *really* hard (frankly too much to ask) for the few core committers to have comprehensive understanding of all of Drupal at this level.
* Makes MAINTAINERS.txt actually mean something.
* Rewards solid effort from community members.
etc...
I think at the very least it's a concept worth pursuing. -- James Walker :: http://walkah.net/ :: xmpp:walkah@walkah.net
James Walker wrote:
On 21-Feb-08, at 7:57 AM, Moshe Weitzman wrote:
FYI, what Karoly proposes is exactly how the Mozilla project works. Each patch needs a "super review" from a fixed (but large) list of reveiwers. See http://www.mozilla.org/hacking/
Those who have bothered to stick around and listen long enough while I ramble will know that I'm actually in favor of this kind of approach. I think it has several benefits :
* Clear responsibility - in case of questions, complications, etc - the "buck stops" at a defined point for resolution.
* Allows for "domain experts" - very frequently patches come in that can have tricky, subtle implications for some of the more complicated systems (menu, formapi, search etc etc etc) and it's *really* hard (frankly too much to ask) for the few core committers to have comprehensive understanding of all of Drupal at this level.
But we already have this, albeit more informally: - OpenID patches don't get committed until you've looked them over. - Actions patches don't get committed until John VanDyk has chimed in. - chx or pwolanin need to look at menu system patches. - Jeff Eaton or chx need to approve FAPI patches. - Barry or Frando need to thumbs-up Schema API changes. - merlinofchaos and dvessel are in charge of theming-related changes. - quicksketch is the Drupal JS master. And so on. MAINTAINERS.txt /already/ means something. The vast majority of patches, however, are not directly related to these types of low-level changes that require specific domain knowledge; they're string fixes, or usability improvements, or logic fixes, or simple new features, and so on. Being able to rely the "wisdom of crowds" in the larger community to handle tasks like reviewing patches and changing issue statuses is a /good/ thing. Further, I think Mozilla's model works because of the Mozilla Foundation, and the Mozilla Corporation. MoFo directly gets involved in setting directions and policies of development and MoCo has employees who are paid to oversee certain aspects of the code base. Drupal, by contrast, is a flat-hierarchy, bazaar-style project, with no one entity controlling the direction or governance of the project's development. The Drupal Association is removed from setting this type of direction, by design. It's the textbook case of herding cats. ;) Therefore, we need a completely different model: it's 100% in our best interest to *spread around* the domain knowledge and development process as much as possible, rather than entrusting it to a small handful of people. We have absolutely no guarantees that domain experts will remain as such. merlinofchaos might leave the Drupal community to become a world-renowned novelist, or quicksketch might take off and go backpacking around Europe for a year. When our menu system maintainer went AWOL, our only recourse was to start over completely over from scratch again. We should do everything we can to ensure that this does /not/ happen again. -Angie
Angie, excellent post. I reviewed a patch earlier today, http://drupal.org/node/223549. It's a no-duh usability patch that I highly approve of, but I hadn't yet looked at the code, and even if I had, I'm not a coder by any means. So I said, yep, I wish I could mark this RTBC but I can't. But I looked at the code for the heck of it, and I saw that it was a one word change- After to Before. If that's all it takes, it's an RTBC by my standards, so I marked it RTBC. I may be wrong on that, but if I don't have the ability to mark that RTBC, then that's irrational. It's a simple change that was not getting a lot of attention, and Usability is a high priority in D7. What's the point of contributing if you can't make that simple change? Sorry for the rant. On Thu, Feb 21, 2008 at 1:37 PM, Angela Byron <drupal-devel@webchick.net> wrote:
James Walker wrote:
On 21-Feb-08, at 7:57 AM, Moshe Weitzman wrote:
FYI, what Karoly proposes is exactly how the Mozilla project works. Each patch needs a "super review" from a fixed (but large) list of reveiwers. See http://www.mozilla.org/hacking/
Those who have bothered to stick around and listen long enough while I ramble will know that I'm actually in favor of this kind of approach. I think it has several benefits :
* Clear responsibility - in case of questions, complications, etc - the "buck stops" at a defined point for resolution.
* Allows for "domain experts" - very frequently patches come in that can have tricky, subtle implications for some of the more complicated systems (menu, formapi, search etc etc etc) and it's *really* hard (frankly too much to ask) for the few core committers to have comprehensive understanding of all of Drupal at this level.
But we already have this, albeit more informally: - OpenID patches don't get committed until you've looked them over. - Actions patches don't get committed until John VanDyk has chimed in. - chx or pwolanin need to look at menu system patches. - Jeff Eaton or chx need to approve FAPI patches. - Barry or Frando need to thumbs-up Schema API changes. - merlinofchaos and dvessel are in charge of theming-related changes. - quicksketch is the Drupal JS master.
And so on. MAINTAINERS.txt /already/ means something.
The vast majority of patches, however, are not directly related to these types of low-level changes that require specific domain knowledge; they're string fixes, or usability improvements, or logic fixes, or simple new features, and so on. Being able to rely the "wisdom of crowds" in the larger community to handle tasks like reviewing patches and changing issue statuses is a /good/ thing.
Further, I think Mozilla's model works because of the Mozilla Foundation, and the Mozilla Corporation. MoFo directly gets involved in setting directions and policies of development and MoCo has employees who are paid to oversee certain aspects of the code base.
Drupal, by contrast, is a flat-hierarchy, bazaar-style project, with no one entity controlling the direction or governance of the project's development. The Drupal Association is removed from setting this type of direction, by design. It's the textbook case of herding cats. ;)
Therefore, we need a completely different model: it's 100% in our best interest to *spread around* the domain knowledge and development process as much as possible, rather than entrusting it to a small handful of people. We have absolutely no guarantees that domain experts will remain as such. merlinofchaos might leave the Drupal community to become a world-renowned novelist, or quicksketch might take off and go backpacking around Europe for a year. When our menu system maintainer went AWOL, our only recourse was to start over completely over from scratch again. We should do everything we can to ensure that this does /not/ happen again.
-Angie
-- Sincerely, Michael
Uh oh, angie... ;-) On 21-Feb-08, at 1:37 PM, Angela Byron wrote: > James Walker wrote: >> On 21-Feb-08, at 7:57 AM, Moshe Weitzman wrote: >>> FYI, what Karoly proposes is exactly how the Mozilla project works. >>> Each patch needs a "super review" from a fixed (but large) list of >>> reveiwers. See http://www.mozilla.org/hacking/ >> Those who have bothered to stick around and listen long enough >> while I ramble will know that I'm actually in favor of this kind >> of approach. I think it has several benefits : >> * Clear responsibility - in case of questions, complications, etc >> - the "buck stops" at a defined point for resolution. >> * Allows for "domain experts" - very frequently patches come in >> that can have tricky, subtle implications for some of the more >> complicated systems (menu, formapi, search etc etc etc) and it's >> *really* hard (frankly too much to ask) for the few core >> committers to have comprehensive understanding of all of Drupal at >> this level. > > But we already have this, albeit more informally: > - OpenID patches don't get committed until you've looked them over. > - Actions patches don't get committed until John VanDyk has chimed in. > - chx or pwolanin need to look at menu system patches. > - Jeff Eaton or chx need to approve FAPI patches. > - Barry or Frando need to thumbs-up Schema API changes. > - merlinofchaos and dvessel are in charge of theming-related changes. > - quicksketch is the Drupal JS master. > > And so on. MAINTAINERS.txt /already/ means something. Well.... *sorta*. If we're using me as an example, yes, it's been mostly true for OpenID so far - definitely not true for blogapi ... it's been broken several times over the years by patches I missed on their way in. Perhaps I'm a bad maintainer (most would argue I am), but I think we could do a lot to help maintainers out. > The vast majority of patches, however, are not directly related to > these types of low-level changes that require specific domain > knowledge; they're string fixes, or usability improvements, or > logic fixes, or simple new features, and so on. Being able to rely > the "wisdom of crowds" in the larger community to handle tasks like > reviewing patches and changing issue statuses is a /good/ thing. To a point. Never would I suggest we remove the wisdom of the crowds... everyone should be encouraged to submit and review patches. Currently, however, we have a very narrow gate (Gabor and Dries for D6) for stuff that actually goes in. What I'm suggesting (or envisioning) is actually widening that gate a bit - by expanding to a super-review tier (that includes MAINTAINERS.txt - and likely others) ... to help ease the burden of the core committers in verifying that an RTBC is *really* RTBC. > Further, I think Mozilla's model works because of the Mozilla > Foundation, and the Mozilla Corporation. MoFo directly gets > involved in setting directions and policies of development and MoCo > has employees who are paid to oversee certain aspects of the code > base. Yes, Mozilla's model is different - based on MoFo & MoCo - but the organizational principles don't dictate or necessitate incorporated bodies. Look at the typical standards bodies - or the ASF - or other models that have similar structures of hierarchy with more 'consortium' type models. Also, mozilla super reviewers are not necessarily MoCo employees (I haven't looked at the list in a while, but IIRC, most of them aren't). > Drupal, by contrast, is a flat-hierarchy, bazaar-style project, > with no one entity controlling the direction or governance of the > project's development. The Drupal Association is removed from > setting this type of direction, by design. It's the textbook case > of herding cats. ;) Drupal isn't a flat-hierarchy. Again, see above. > Therefore, we need a completely different model: it's 100% in our > best interest to *spread around* the domain knowledge and > development process as much as possible, rather than entrusting it > to a small handful of people. We have absolutely no guarantees that > domain experts will remain as such. merlinofchaos might leave the > Drupal community to become a world-renowned novelist, or > quicksketch might take off and go backpacking around Europe for a > year. When our menu system maintainer went AWOL, our only recourse > was to start over completely over from scratch again. We should do > everything we can to ensure that this does /not/ happen again. I'm not advocating single points of failure... again, I would advocate additional redundancy. The key motive here being essentially formalizing the structure that you alluded is already in place - with the end result being formal help, and responsibility distribution outside the current core committers. Truth is, we're not so different. Mozilla has *thousands* of contributors ... they're an older, bigger project... and I think we can stand to learn from them as well as others out there. Shoulders of giants 'n' all that :-) -- James Walker :: http://walkah.net/ :: xmpp:walkah@walkah.net
If we leave the RTBC permissions as they currently are, a confirmation step could reduce the number of untested patches that accidentally make it to the RTBC status. This additional step could be added conditionally based on user roles so it wouldn't impact usability for those who go through the process of marking patches as RTBC multiple times a day. The confirmation step could look like this: By marking this patch as RTBC (ready to be committed), you confirm that: - The patch applies to the latest version of the code - The patch gives the desired result - The code is clean - The code is well-documented If you are unsure about any of the points above, do not change the status of the patch. [Keep current status] [Set status to RTBC]
On Thu, Feb 21, 2008 at 8:01 PM, James Walker <walkah@walkah.net> wrote:
Uh oh, angie... ;-)
On 21-Feb-08, at 1:37 PM, Angela Byron wrote:
James Walker wrote:
On 21-Feb-08, at 7:57 AM, Moshe Weitzman wrote:
FYI, what Karoly proposes is exactly how the Mozilla project works. Each patch needs a "super review" from a fixed (but large) list of reveiwers. See http://www.mozilla.org/hacking/
Truth is, we're not so different. Mozilla has *thousands* of contributors ... they're an older, bigger project... and I think we can stand to learn from them as well as others out there. Shoulders of giants 'n' all that :-) -- James Walker :: http://walkah.net/ :: xmpp:walkah@walkah.net
Yesterday I found this post of Stefano Mazzocchi ( http://www.betaversion.org/~stefano/linotype/news/104/) very inspiring and I think his reflections (and Linus words too) on the difference between CVS/SVN and GIT and implications on community governance are very appropriate to issue raised on this thread. It's no matter of religion wars on versioning tools (a knife is a knife no matter you can kill someone or enjoy some pate' on a croasted bread) it's the fact that some model (delegation of the autority) could be better than other (MAINTAINERS.txt). Maybe Linux is too far from our little blue word but I guess that if we want to preserve the "passion" of this community we wouldn't move through a more beuarocratic system. Well, I don't have any idea which solution could work, but still I think the debate should take this point very seriously. best ema -- Emanuele Quinto - www.bitvark.it ------------------------------------------------------------------------------------------------------------------ My mother used to say to me, "Elwood" - she always called me Elwood - "In this world, Elwood, you must be oh-so smart, or oh-so pleasant." For years I was smart. I recommend pleasant, and you may quote me. ------------------------------------------------------------------------------------------------------------------
Angela Byron wrote:
they now start PMing and e-mailing this small handful of people "RTBC my patch!!" which I can tell you right now is going to cause even further grating of nerves. :P
Followed by somebody suggesting that there be a "Ready to Be RTBCed" to "ease the suffering". :| RTBRTBCalafragalisticexpialadocious andre
On Thu, Feb 21, 2008 at 7:49 AM, Karoly Negyesi <karoly@negyesi.net> wrote:
Onwards, I would like to see a small handful people (me not necessarily included!) to be able to RTBC core patches. As we go on, more and more people will misunderstand what this means. A nice pyramid, a few people on top deciding the patches, a few more culling the flood of patches for them. As there is already a permission for this, a little hacking (which I am willing to do) surely would make it possible to have it just for project 3060.
No thanks. I want everyone to be able to approve patches. When I need help from an expert, I'll ask for help -- as I've done on many occasions. We should focus on removing barriers and red tape, not on introducing more. -- Dries Buytaert :: http://buytaert.net/
On Fri, Feb 22, 2008 at 9:56 PM, Dries Buytaert <dries.buytaert@gmail.com> wrote:
On Thu, Feb 21, 2008 at 7:49 AM, Karoly Negyesi <karoly@negyesi.net> wrote:
Onwards, I would like to see a small handful people (me not necessarily included!) to be able to RTBC core patches. As we go on, more and more people will misunderstand what this means. A nice pyramid, a few people on top deciding the patches, a few more culling the flood of patches for them. As there is already a permission for this, a little hacking (which I am willing to do) surely would make it possible to have it just for project 3060.
No thanks. I want everyone to be able to approve patches. When I need help from an expert, I'll ask for help -- as I've done on many occasions. We should focus on removing barriers and red tape, not on introducing more.
I agree. Removing RTBC is not a good solution. There are plenty of unusual issue status combinations that should not be used most of the time: - RTBC with only one user posting - Features on 6.x and earlier - Anything on 4.7.x and earlier - Patches without proper patches What I would like to see is warnings on viewing those issues, and a forced preview with warning before submitting those issues. Most good developers know the rules and follow them, so nothing needs to be forced since there are exceptions to rules. New developers can not be expected to know everything right-away, so the instructions should be contextually shown when needed. Ideally, this would all work through a configuration tool so we can rearrange the rules as-needed. -- Neil Drumm http://delocalizedham.com
participants (13)
-
Andre Molnar -
Angela Byron -
Daniel F. Kudwien -
Dries Buytaert -
emanuele quinto -
Florian Loretan -
James Walker -
Karoly Negyesi -
Michael F -
Moshe Weitzman -
Neil Drumm -
Omar Abdel-Wahab -
Robert Douglass