Strategies for far-reaching core patches

Created on 7 September 2011, over 13 years ago
Updated 5 July 2023, over 1 year ago

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.

Schedule patches

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.

Proposed resolution

On August 4, 2011 at 9:43am, Dries β†’ set a precedent by posting:

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 .

In order to promote harmony and reduce conflict, this policy should be discussed and standardized.

Remaining tasks

πŸ“Œ Task
Status

Closed: outdated

Version

9.5

Component
BaseΒ  β†’

Last updated 43 minutes ago

Created by

πŸ‡¬πŸ‡§United Kingdom catch

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

Production build 0.71.5 2024