Planet Drupal

Diaries of a Core Developer

I am posting this in light of webchick's insightful series of blog posts, a more recent revival of the topic, and my own utter frustration. I understand this subject has been brought up before with no change so I was a bit reluctant to bring it up again, but I think it is important. Personally I have been so fed up with core development that I have taken over a month off from serious core development. Given that other developers express this sentiment from time to time and that you can feel the discontent in the community (granted some are perfectly happy) I think things need to change.

Keep in mind I have reworked this quite a bit over the course of several weeks and sat on the whole subject for some time to ensure that I had my perspective right. Not all my reasons, experiences, and thoughts are included as I tried to focus on the most important.

I am going to present some of my own experiences and my personal mindset when working on Drupal core and the reasons behind it. I will not point to specific issues or people.

General patch workflow/mindset
In general when I submit a patch I expect there to be several revisions of the patch before it is finally committed. I think that is a healthy sign and the patch ends up better at the end. Another thing I expect is the patch to take 1-4 weeks to get committed on average with a few exceptions on both both ends of the spectrum. It usually takes a week or two to get a review and bring it to RTBC status and then anywhere from from a day to three weeks to be committed.

Given that most patches are not standalone, that is to say they are part of a larger set of changes, it can take an extremely long time to get any sort of major change into core. A shortcut to this is the controversial and feared "large patch." There are two reactions to such a patch. If the patch is a cleanup/refactoring (ie. unexciting) or for whatever whim it may be rejected as "two large to review" and the creator is told to split it into smaller patches. In many cases this is virtually impossible and if possible will take months to get them all the patches committed, not to mention the overload of work to split the patch up. I have let a number of large cleanups of the testing system die for this reason and reworking smaller patches is a joke. To accomplish any major overhaul can easily take 10 patches at ~3 weeks a piece it is impractical. Which means SimpleTest still has issues with static variables, after several attempts, among other things.

There is a more general implication of this, I do not even bother rolling several patches at a time to the same system because they will sit so long that I will be forced to waste time continually re-rolling them. I have attempted this on several occasions hoping I could get things to move a bit faster, but became utterly frustrated. Additionally, I personally ignore minor changes and cleanups, because I do not feel like going through the hassle of getting an obvious patch committed.

Responsibility without authority
Another problem that I have run into is architecture decisions. These decisions tend to result in massive wastes of time from stagnation or constant rewriting. As Larry Garfield noted (excerpt):

Schema API is right now lagging far behind the rest of the DB layer. It's API is old-school, it has limited capabilities, it's nearly useless outside of update hooks, and it's totally out of sync with the rest of the system. In April, I posted an issue laying out two different ways we could take Schema API. I asked "OK, so which do we want to do?" because as Matt says above no one wants to spend dozens of hours on one approach only to be told later "nah, let's do the other one instead". You'd charge a client extra to tell you that, but I can't bill Dries for my core development time. :-)

That was in mid-April.

I personally asked Dries for feedback by email.

I personally asked webchick for feedback in IRC.

I emailed the dev list pointing people to the issue.

I got involved in a lengthy discussion on the dev list, asking "so as DB Maintainer, what the heck do I actually DO? Do I have the authority to just make this decision and not have it overruled on a whim later?"

There are always multiple ways of doing something and many times people do not comment until they see something they do not like. A problem is that in many cases an entire rewrite is required to satisfy them when many times it is just not necessary. Having more people with authority on large decisions will help alleviate this stress.

Bestow responsibility
Now some may say, "So what are you suggesting? Give more authority to the 'Maintainers'?" People may then look at various core developers, many of whom hold "maintainer" positions and point at the patches that they marked as RTBC that were not. I know I am guilty of this and many other developers as well. My reasoning is that a patch can be debated for centuries before it is marked RTBC and there will still be complaints made so I would rather get those complaints early on rather then later (after waiting a week+ for a review in RTBC queue of course). Better to get my patch's place in line than debate it before an initial core committer review. I can say for a fact I review my code more thoroughly for my contrib modules since I have commit access to them. I am sure the same would hold true if more core developers were given commit access.

Others may say that it is good to have patches thoroughly reviewed by people who are intimately involved with Drupal (core committers) as they will ensure the patches are better. Granted many reviews I receive in such a manor are very thorough and well thought out, but there are plenty of terrible patches that get by which I can spot flaws with. Everyone is human and no one can catch everything. Then of course I must wait weeks to get the fix in for those patches. Of course the argument above is only valid if you assume a larger commit group means that unqualified developers will review patches. If you take a look at the MAINTAINERS.txt file in core I think you will find that quite the opposite is true.

Another note on the reviews. Many times a patch will improve a system or bit of code dramatically and there are further improvements planned. In the interest of incremental changes (and Extreme Programming) it makes sense to commit the code even if it is not as good as it could be. When writing a follow-up patch one will most likely find shortcomings of the previous patch and clean it up along the way. This process is much faster and much more effective. This allows the majority of code to be better overall instead of just select areas that are reviewed with intense scrutiny.

Conclusion
From what I can see this aged process of core development is holding back Drupal and asking for a fork. It is easily conceivable that enough people will get annoyed (especially as Drupal continues to grow) with the slow speed of development. I do not want to see that happen and I would like to see Drupal get better, faster.

I do not pretend to have all the answers, but I can present the issues I have encountered and logical responses as I see them.

Firstly, regardless of what you look at: patches sitting in RTBC queue, developers sitting for months waiting for a decision, or the fact that a "code slush" is even needed it is clear to see that two committers is not enough. Many people will argue we need more reviewers, but one must look at the RTBC queue and see that would only make things worse. Given that systems like the Schema API are not up-to-date with DBTNG, even though that was introduced very early on in D7 cycle, you know we have issues. I can say for a fact that I was just totally burned out after rewriting the internal browser patch on every whim and as such it is not implemented anywhere in core and I can only imagine that this holds true for numerous systems.

Proposal
I do not think that adding one, even two committers is the solution. We need a committer for each system (like every other open source project I know and Drupal contrib), who knows that system inside and out. Given that we already have a list of those people, who have no authority at the moment, the fix seems easy to implement. I do not mean that the database maintainer should only have access to /includes/database as that leads to the cases where two lines need to be changed elsewhere or perhaps all modules need to be updated for database API changes and now the maintainer is powerless. Instead maintainers should be watched over by the community and possibly 1-2 overall maintainers, like we have now.

The maintainers then have final say over their respective areas, but should take into consideration discussion with the community and of course the overall maintainers. In the situation Larry described, he would be free to decide on a path and implement it given that no feedback was provided and noone presented any valid reasons against the approach.

Defining some sort of overall roadmap, like what core is to be makes sense and that may fall more in the realm of the overall maintainer(s) and the community. The maintainers are then bound within that roadmap.

I am sure there are additional ways to improve the core development cycle, but I believe the above changes are crucial. If nothing changes, I suspect I will focus on hacking contrib after the D7 bug cycle.

Thoughts
I appreciate thoughts and comments on this issue, but I would appreciate it if only those "actively involved" in Drupal core development would comment as those are the individuals most effected by this.

Tags:

Fresh SimpleTest backport - 2.9

Considering:

  • the last official backport was April 23 2009
  • a number of very cool features have been added in core in addition to incremental changes
  • people have been asking
  • Drupal 7 is in "code slush"

it seems appropriate to perform another backport. I finished the bulk of the backport during Drupalcon Paris and have been tweaking and fixing bugs from feedback since then.

In order to ensure that all the new features from Drupal 7 were available it was necessary to create a patch against Drupal 6 core which needs to be applied before installation, as described in INSTALL.txt.

Please update and report any issues in the queue and have fun with the new features and proper error reporting!

test run

Drupal 7: debug() and SimpleTest->verbose()

Recently, I have made two major improvements to debugging in Drupal 7, the addition of a general debug function and a verbose mode for SimpleTest. The two additions make it much easier to debug problems quickly through the use of a consistent method. Take a look at what chx said via twitter:

Writing #drupal code? Check the new function debug(). Writing #drupal tests? Check $this->verbose(). And debug() works too. AWESOME!

General debug function
The general debug function can be used at any point after Drupal is bootstrapped, although the limitation may be removed in the future. The function provides a very simple wrapper to dump data through the use of var_export() and print_r(). When used normally it will display data based on the "Logging and errors" settings provided in Drupal 7 core. If using a dev version the debug information will be displayed using drupal_set_message() as shown in the screenshot below.

debug normal

The exciting part about the new debug() function is that it also works during testing. The debug() function can be placed inside the test itself or in any other part of Drupal and it will be picked up and displayed in the test results as shown below.

debug test

SimpleTest verbose mode
Another exciting new debugging tool that is extremely useful when writing tests is the new verbose mode for Drupal 7 SimpleTest. The verbose mode can be enabled on the SimpleTest settings page.

verbose setting

Once enabled SimpleTest will automatically record the page as it was seen by the SimpleTest browser after each drupalGet() and drupalPost() call. A link is then placed in the test results that will display the page the browser saw and some meta data related to the request.

verbose link

Page 1

verbose page1

Page 2

verbose page2

Manual verbose
In addition to the automatic message provided by SimpleTest custom verbose data may be dumped using DrupalWebTestCase->verbose() which can be used in a test as shown.

<?php
$this
->verbose($data);
?>

If the data to be dumped in not available in the test, but in the code being tested a function is provided that may be accessed by including the DrupalWebTestCase as shown below.

<?php
require_once drupal_get_path('module''simpletest') . '/drupal_web_test_case.php';
simpletest_verbose($data);
?>

Summary
By adding these debugging tools to Drupal 7 the developer experience involved in writing a test has been greatly improved. These methods can still be improved and as such please feel free to file issues in the Drupal 7 SimpleTest issue queue. Also note that this work was sponsored by Acquia as part of my Summer Internship.

openSUSE one-click for LAMP and Drupal

I decided to play around and see if I could create an YaST Meta Package for:

  • LAMP stack
  • Drupal/SimpleTest required PHP extensions
  • phpMyAdmin

After a quick read of the Build Service/Tutorial and a talk with cyberorg in IRC (#suse/#opensuse-buildservice) I was able to upload my pattern to the openSUSE build service.

It was quite simple as I expected since openSUSE's build service and such seem to be quite nice. I have plans to fix up some of my other Drupal packages, add more, and work on personal projects. If you use openSUSE and give it a try, let me know if it works or you have any feedback.

My repository is available at:

Help D7 SimpleTest maintainer and usablity expert get to Drupalcon Paris 2009

The scholarships provided by the Drupalcon Paris team will not cover all the accommodation expenses. Bojhan and myself (boombatower) have decided to stay together and share the expenses. In order to make it to Paris we need to raise at least $500 USD, otherwise I will not be attending.

Take a look at our user pages to get an idea of the work we have done for Drupal.

I plan to give two sessions: Introduction to testing with Drupal: SimpleTest and Sept 2nd, Drupal 7 release party. How continuous integration testing made this plausible.

Bojhan has two presentation proposals as well: Building blocks for your module's UI and How Open Design will drive Drupal 8.

We appreciate your donations and hope to accomplish much at Drupalcon Paris.

Update restore pattern

Recently I have been working on confirming that the Project Issue File Review (PIFR) and Project Issue File Testing (PIFT) update paths from 1.x to 2.x work properly. I was attempting to do so with a copy of the live data from testing.drupal.org and drupal.org. Doing so would ensure that the somewhat delicate upgrade path functioned properly.

I ran into the issue of resetting the data after a failed update attempt. Doing so is not a quick and simple task, especially for the PIFR upgrade which requires renaming the old tables (I may move into udpate path itself) so the data can be converted and placed in the new schema. To solve the problem I developed the following script, or pattern, that can be used to reset for another update attempt. The first script is the one used for the PIFT update which is much simpler. The pift_data.sql contains the live data from drupal.org and similar with the PIFR update.

I placed the scripts in index.php, just as I do with all my scripts, after the <?php drupal_bootstrap(DRUPAL_BOOTSTRAP_FULL); ?> line. The scripts can then be ivoked by appending ?restore=true to the URL to your site. Obviously I commented out the one I was not using.

I found that having these scripts helped ensure that I tested the update path extensively and as such they seem like a good thing to share.

PIFT update reset script

<?php
if (isset($_GET['restore'])) {
  
header('Content-Type: text/plain');
  
ini_set('display_errors'1);
  
set_time_limit(0);

  echo 
"resetting...\n";

  
db_query('TRUNCATE TABLE {watchdog}');

  
db_query("UPDATE {system}
            SET schema_version = %d
            WHERE name = '%s'"
6000'pift');

  
$tables = array(
    
'pift_data',
    
'pift_test',
    
'pift_project',
  );
  
$ret = array();
  foreach (
$tables as $table) {
    if (
db_table_exists($table)) {
      
db_drop_table($ret$table);
    }
  }

  echo 
"importing d.o dataset...\n";

  
$parts parse_url($db_url['default']);
  
$command "mysql -u{$parts['user']} -p{$parts['pass']} " substr($parts['path'], 1);
  
exec($command ' < /home/boombatower/download/pift_data.sql');

  echo 
"finished...\n";

  
print_r($ret);

  exit;
}
?>

PIFR update reset script

<?php
if (isset($_GET['restore'])) {
  
header('Content-Type: text/plain');
  
ini_set('display_errors'1);
  
set_time_limit(0);

  echo 
"resetting...\n";

  
db_query('TRUNCATE TABLE {watchdog}');

  
db_query("UPDATE {system}
            SET schema_version = %d
            WHERE name = '%s'"
6000'pifr');

  
$tables = array(
    
'pifr_branch',
    
'pifr_client',
    
'pifr_file',
    
'pifr_log',
    
'pifr_project',
    
'pifr_result',
    
'pifr_result_detail',
    
'pifr_result_detail_assertion',
    
'pifr_test',
    
'pifr_file_old',
    
'pifr_result_old',
    
'pifr_server',
    
'pifr_log_old',
  );
  
$ret = array();
  foreach (
$tables as $table) {
    if (
db_table_exists($table)) {
      
db_drop_table($ret$table);
    }
  }

  echo 
"importing t.d.o dataset...\n";

  
$parts parse_url($db_url['default']);
  
$command "mysql -u{$parts['user']} -p{$parts['pass']} " substr($parts['path'], 1);
  
exec($command ' < /home/boombatower/download/tdo.sql');

  echo 
"renaming...\n";

  
db_rename_table($ret'pifr_file''pifr_file_old');
  
db_rename_table($ret'pifr_result''pifr_result_old');
  
db_rename_table($ret'pifr_log''pifr_log_old');

  echo 
"importing pifr_server schema...\n";

  
exec($command ' < /home/boombatower/download/pifr_server.sql');

  echo 
"finished...\n";

  
print_r($ret);

  exit;
}
?>

Tags:

Firefox userContent.css for drupal.org issue queue

After seeing a reference to http://userstyles.org/styles/11133 by tha_sun in IRC I went ahead and played with it a bit. I ended up with a very simple version that that just removes the blocks and makes the issue table full width. While I was at it I customized Google.

If you do not know where to put this checkout, per-site custom CSS in Firefox.

@namespace url('http://www.w3.org/1999/xhtml');
 
@-moz-document url-prefix('http://drupal.org/project/issues/') {
  #spacer {}
 
  #contentwrapper {
    background: none !important;
  }
 
  #main {
    margin-left: 5px;
    margin-right: 5px;
  }
 
  #threecol, #content, #squeeze {
    width: 100% !important;
    margin: 0 !important;
    padding: 0 !important;
  }
 
  .sidebar .block {
    display: none !important;
  }
}
 
@-moz-document url('http://www.google.com/') {
  #spacer {}
 
  #ghead, td[align='left'], td[width='25%'], #body > center > font, input[name='btnG'], input[name='btnI'], #footer {
    display: none;
  }
 
  #main > center {
    margin-top: 250px;
  }
}

Acquia internship

Acquia logo

This Summer I will be working part-time as an intern for Acquia. I am very excited to be working with Acquia and having the chance to spend more time improving things that I have interest in. To clarify I will be working on projects that benefit the entire Drupal community. The items I will be working on are improvements to projects I have either started or that I am heavily involved with.

During the discussion of the internship I came up with the following goals that were then prioritized by Dries.

Primary goals

  • Finalize testing of contributed modules and Drupal 6.x projects/core.
  • Add executive summary of test results on project page.
  • Extend the SimpleTest framework so we can test the installer and update/upgrade system.
  • Improve and organize SimpleTest documentation
  • Work on general enhancement of Drupal 7 SimpleTest.

Secondary goals

  • Provide on-demand patch testing environment for human review of patches.
  • Finish refactoring of SimpleTest to allow for a clean implementation of "configuration" testing.
  • Analyze current test quality and code coverage, and foster work in areas requiring attention.

I will post updates on some of the more interesting items as they are accomplished. Additionally, I would like to give a special thanks to Kieran Lal for his mentoring and help in finding me sponsorship.

FOLLOW UP:
To clarify I will still be participating in Google Summer of Code 2009, which was explicit in my agreement with Acquia.
Follow up post by Dries.

Pages

Subscribe to RSS - Planet Drupal