DrupalCon Baltimore: 161 sessions, many voices, infinite possibilities. Earlybird rate ends Friday.
Strategies for big core patches.
There have been several discussions in the past about the difficulty of getting large patches into core. And between D8 initiatives and everyone's general plans this is going to be coming up a lot as 8.x gets moving.
Here are a few ideas, that came from a chat with catch, chx, Dries, and webchick.
Dries has scheduled the /core move patch for November 1st. We could potentially do that for any patch that is going to move thousands of lines of code around. This means people rolling the patch know they have a specific week it needs to be ready for. And everyone else knows that their patches are going to get broken after that week. In both cases it should help cut down on 're-roll hell', which is easier with git but has not gone away. It also could present an opportunity for wide-spread clean-up patches like renaming/moving files, fixing coding standards, documentation standards, etc. since everyone's patches are going to break anyway.
Temporary backwards compatibility layers
dbtng went in with a backwards compatibility layer so that existing calls to db_query() would continue to work temporarily. This allowed the work of converting everything to the rest of the API to happen over time (iirc it took a year or 18 months to actually remove the bc layer).
http://drupal.org/node/81461 is a much smaller change but the current version of the patch there also contains a bc layer for this reason - keeps the patch at a reviewable size and saves breaking things all at once.
This is not how we have traditionally handled things in the past so could use some discussion. We need to figure out what happens with the follow up work (is it major/critical tasks?). Should we use @deprecated in phpdoc on the old functions to make it clear to people chasing HEAD? Could we throw E_DEPRECATED errors maybe (if we can avoid tests failing due to those).
Temporary hiding of failing tests
In a worst-case scenario, if you can't leave a backwards compatibility layer in while doing major refactoring, another option is to silence the failing tests in optional modules that are not enabled by the standard profile, either by commenting them out or by adding an expectedFail() method to the Testing module. Each of these would require opening a critical task to put back, and thus would go against our thresholds. While this would likely hold up further refactoring, that's kind of the point. And in almost all cases, making similar changes in several modules is something we could tag as "Novice" and point new core contributors at, so they should get cleaned up relatively easily.
If it's broken, roll it back
Our current approach if an unexpected regression is found or 'HEAD is broken' is to all scramble and try to fix it. Instead we could try immediately rolling back the patch (assuming it's not in a stable release yet), fixing whatever the regression is in the issue itself, then re-committing along with the regression tests. This would shift the burden of responsibility from critical core bug, back to just being dealt with in the issue.
This requires a bit of a cultural change, to be accepting that if your patch rolls back it doesn't mean it's not getting in; it just means it can't get in until it's actually ready. :)
The goal of this issue is to create a documentation page in the handbook that outlines the process someone wanting to do major refactoring to Drupal core should go through. Let's discuss strategies and come up with a plan.
I've looked at this and I'm comfortable with the patch. Given that it may break a lot of patches, I'd like to propose that I commit this on November 1st, 2011.
Other senior developers, committers, and approvers have followed this example by delaying or pre-scheduling large patches that would make many patches need rerolls.
In order to promote harmony and reduce conflict, this policy should be discussed and standardized.
- Documentation page
Whatever is decided here should be published as a documentation page for reference. This documentation page should be linked from strategic places.
- Standard workflow
Provide a standard workflow for coordinating, announcing, and committing such patches. For example, It should be possible for developers to check whether their patch is likely to conflict with a pre-approved patch that is waiting on a scheduled commit. It has been proposed that pre-scheduled core commits be published on the Drupal core and/or the Issue Triage event calendars.
- Threshold guidelines
Some feel that published threshold guidelines may help decide whether a patch is big or invasive enough to warrant a pre-announced and pre-scheduled commit. Others think that patches of this size are few enough that a "common sense" approach is all that is needed.