So it seems Larry Garfield (Crell) and me nailed a serious issue in Jimmy Berry's (boombatower) post about Drupal core development, where even chx's idea of sub-system maintainers not being able/allowed to commit their own patches would not help, because the lack of communication and coordination remains. In fact, adding more people with commit access would vastly increase the problem, and now that I think about it, this is exactly what Dries mentioned to me in person when I discussed more core committers with him in Paris.

Core (branch) maintainers need to talk about and coordinate what they are doing very frequently. It is not like each one is able to act completely independently, because that would result in a complete nightmare. In my recent Challenges for Drupal post I already tried to clarify what the actual job of core maintainers is, after having some insightful conversations with Dries and webchick.

Hence, if we had core branch maintainers + sub-system maintainers, all with commit access, then the group of people that would need to coordinate all of their actions (before committing something) would bump from 2 to 10 or whatever, which would not work out.

When I wrote my follow-up on Jimmy's blog, I tinkered about my past and current work and involvement in Drupal core development, which is mainly doing patch reviews for all kind of patches. The conclusion was that this work and thereby my actual role (without intention) might be called "code gardener" (read the long version). As such, I'm spotting awkward code, ill logic, wrong implementations/integrations with other sub-systems, completely missing or wrong documentation, or even entirely wrong ideas and approaches, as early as possible. All of that has the sole purpose of trying to get all kind of patches into "real" RTBC state before the responsible core maintainers look at it.

I'm not sure whether I fully agree with all details of Crell's perspective on the hierarchy and core contributor roles. Unleashing my mind, I'd probably put it that way:

  1. Feature contributors

    Work on patches touching high-level features, i.e. CMS functionality; changing data, variable default settings, forms, markup, whatever.

  2. Framework contributors

    Work on patches touching low-level APIs and sub-systems, i.e. framework functionality; adding or changing/improving/extending/cleaning up APIs, most often having a rather large impact on the overall system and product (because all features need to be changed accordingly).

  3. Sub-system maintainers (specialists)

    Help contributors to get their patches right with regard to the sub-system they maintain, but also formulate, communicate, and coordinate visions and goals. And of course, also work on massive sub-system improvements (rewrites etc) on their own.

    Specialists are the group of people who really drive product innovation. Of course, taking up influences from framework contributors and constantly discussing/sharing vision with core branch maintainers.

  4. Code gardeners (generalists)

    Review and test patches in a general manner, i.e. proper cross-API/sub-system usage, security, logic, coding standards, documentation (especially for API changes), consequences for implementors/implementations, and overall quality. Mainly to decrease the workload for core branch maintainers.

    In general, the work of code gardeners requires in-depth understanding of most/all sub-systems (not necessarily all innards, but at least the fundamentals and more importantly, the ideas and architecture behind each one) and a broad understanding of how the APIs and features are currently used, especially by contributed modules.

    As a matter of fact, generalists have to trust specialists for API changes that touch the innards of a sub-system, because such changes are beyond their horizon (otherwise, they'd belong to the group of sub-system specialists).

    Generalists are the group of people who need to know about all visions, goals, plans, and all recent/pending changes as well as the state-of-the-art APIs, and therefore need to communicate with both, sub-system maintainers and core branch maintainers.

  5. Core branch maintainers

    Do final reviews and are the last resort for catching any issues with a patch. Because they have the responsibility of pushing the button, they need to be able to rely on a trust level to sub-system maintainers and code gardeners. At this point, a patch is not about personal opinions, but about quality instead.

    IMHO, ideally, core branch maintainers should communicate rather offensively with code gardeners and sub-system maintainers in case they found critical issues with a patch after reviewing - i.e. if that happens, then it means that the communication and collaboration before failed, and they absolutely need to drive direction to prevent that another patch runs into the same problem in the future. Only if specialists and generalists know what flies and what doesn't, we are able to increase the pace in our overall workflow.

Yes, that's quite a hierarchy ;) But I think it maps to what we already have in reality. Interestingly, it also maps quite nicely to the Drupal learning curve (which I guess I need to revise based on all these considerations ;).

Of course, those "positions" can be fluent. And it's not that a core branch maintainer couldn't contribute a patch - in fact, if there was better communication and a streamlined workflow, then I could even imagine that a core branch maintainer would also have more time to do something else than just looking at the RTBC queue.

If we would follow this idea, then we would have these concrete tasks at hand:

  • Establish a better communication and collaboration between sub-system maintainers, code gardeners, and core branch maintainers
  • Fill in missing sub-system maintainers in MAINTAINERS.txt (our process only works if there is someone to talk to about something; at least every includes/*.inc file needs at least one, and assigning at least someone for each modules/*.module wouldn't hurt either)
  • Maybe even assign code gardeners in MAINTAINERS.txt?
  • Introduce and use issue tags for all maintenance teams/topics, so maintainers actually have a target.
  • Remove the line titling "THE REST:" from MAINTAINERS.txt, which is the root of one of our maintenance and communication problems.

Thoughts?

Comments

Seems like many people agree with these thoughts after discussion in IRC, so I'd be interested in worries.

(But surely also interested in expressions of agreement, heh)

One problem I just realized - and we indeed have that problem already - is that when a sub-system maintainer AND a code gardener are working on a patch, they will have hard times to find someone for peer-reviews, because already two people of a very small group of people being able to do quality reviews basically cannot really sign off that patch.

Additionally, there is very likely a too high trust-level for any peer-reviewers in the game - because both a sub-system maintainer and code gardener worked on a patch - so how bad and wrong can it really be...? :-/

Hence, that is clearly the scenario where a patch may only get peer-reviews from people that worked on the patch, and core branch maintainers may be involved too early.

Not really sure whether that is really an edge-case scenario or perhaps even happens way too often.

Post new comment

The content of this field is kept private and will not be shown publicly.
  • Allowed HTML tags: <a> <blockquote> <em> <strong> <u> <cite> <code> <ul> <ol> <li> <dl> <dt> <dd> <div>
  • Web page addresses and e-mail addresses turn into links automatically.
  • Lines and paragraphs break automatically.

More information about formatting options

By submitting this form, you accept the Mollom privacy policy.