Problem/Motivation

In #3264122-24: Move all non migration aggregator tests to the module in preparation of removal in d10 @Spokje sought clarification the approach to issue scope taken when we remove a module from core.
This issue is to decide on that scope and document the steps in the handbook.
Further slack discussion for context.

Proposed resolution

Policy - 'Deprecate core module'

  1. Discuss module removal
    1. Open an issue against the Core ideas queue providing justification for removing the module.
    2. Seek approval from product managers.
  2. When the above issue is marked Fixed and the decision is to remove the module from core, then open a core issue 'Meta - Implement deprecation of code module {module}' that will track and coordinate the work. The Meta should have the following steps where each will require a new issue. The first step will also be a Meta because there are several tasks to be done to make sure everything is moved.
    1. Move code and references to the module in core to the module.
    2. Create the contrib project with a stable release.
    3. Deprecate the core module in the current major version.
    4. Remove the module from core in the next major version.
    5. Commit the deprecation patch and the removal patch within 24 hours of each other.
    6. Open an issue against https://www.drupal.org/project/project_composer.
    7. Move core issues for the module from the core queue to the contrib module.
    8. Move all core documentation for the module to the contrib project.
    9. When the issue in https://www.drupal.org/project/project_composer is Fixed update the documentation with the new composer command.

Implementation details

  1. Move code and references to the module in core to the module.

    This step may need a Meta so the work can be scoped sensibly. The following areas need to be addressed. If needed add @todo statements linking to the removal issue.

    1. Move non migration tests to the module.
    2. Move help topics to the module.
    3. Possible issue for removing references to the module from database dumps.
    4. Handle migrations, if the module has migrations.
      • Copy the drupal 6 and drupal 7 test fixtures to the module so they remain independent of any core changes to the fixture.
      • Migration yml and processes that are in the module move with the module to contrib.
      • Move migration tests to the module.
      • Keep all the assertions in core tests related to module as much as possible.
      • Migration yml and processes that are outside the module but support the upgrade path for that module stay in core.
      • The functional tests in migrate_drupal_ui will need to change. Add @todo to remove the module in the removal issue.
      • The drupal 6 and drupal 7 test fixtures stay the same.
  2. Create the contrib project with a stable release.
    1. Once the above items in the meta are complete, create the contrib project. Use a sub-tree split to retain git history.
    2. Ensure tests run successfully.
    3. Check it works and can be installed with composer.
    4. Create a stable release.
  3. Deprecate the core module in the current major version.
    1. Set lifecycle to deprecated, to be deprecated in next major version.
    2. Set lifecylcle_link to the doc page.
    3. Add '@group legacy' to the tests in the module.
    4. The change record for this issue includes a link to the doc page.

    This is postponed on the above children and creation of a stable
    release for the contrib module.

  4. Remove the module from core in the next major version.
    1. Create an issue, 'Remove module from core'.
      • The change record for this issue includes a link to the doc page.
      • Tag this issue 'Needs release note.'
      • Remove the module ;-).
      • Remove references from core/phpstan-baseline.neon.
      • Check for references in @todo.
  5. Commit the deprecation patch and the removal patch within 24 hours of each other.
  6. Open an issue against https://www.drupal.org/project/project_composer.
  7. Move core issues for the module from the core queue to the contrib module.
  8. Move all core documentation for the module to the contrib project.
  9. When the issue in https://www.drupal.org/project/project_composer is Fixed update the documentation with the new composer command.

Remaining tasks

Discuss/decide

User interface changes

API changes

Data model changes

Release notes snippet

Comments

larowlan created an issue. See original summary.

larowlan credited Gábor Hojtsy.

larowlan credited Spokje.

larowlan credited bbrala.

larowlan credited quietone.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Issue summary: View changes
Status: Active » Needs review

🔧 Updating the issue summary with proposed list/steps kindly collated by @quietone and added some additional steps.

➕ Adding issue credit for those involved in the relevant slack discussion.

💬 Please discuss

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Issue summary: View changes
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Issue summary: View changes
quietone’s picture

quietone commented

How is #3 completed? Is is manual or automated?

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented

How is #3 completed? Is is manual or automated?

I think we'd need to ask @drumm if it can be automated

quietone’s picture

quietone commented

Yes, that makes sense.

Looking at #3227033: Remove Quick Edit from core, it is postponed on two issues that don't fit into 1-5. Perhaps a step 0 is needed for researching the module and making children with sensible scope where steps 1-5 are known issues that need to researched. Or something like that to identify all the bits to do before leaping in.

quietone’s picture

quietone commented
Issue summary: View changes

Added 'adding legacy' to tests in the IS.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented

Agree, because they may not all be required in all cases

quietone’s picture

quietone commented
Issue summary: View changes
bbrala’s picture

bbrala
Location Netherlands
commented

Thanks for this issue! Some thoughts;

Not sure how detailed you need to go here.

But there might be references to the module in other places. Like comments in core api.pho files.

Also, documentation might be an issue. As long as the module is in a supported core version one will expect the docs in the core namespace but that can't be forever ofcourse.

The extracted module could refer to core docs until the module is no longer supported in core, then docs could move to a new location. Not sure how we would make sure that that step is followed though.

spokje’s picture

spokje
Primary language Dutch
commented

Agreed with what @bbrala states in #15, there (almost) always will be textual references in comments/core.api.php/whatever, is that sub-issue worthy?

7.2. Set lifecylcle_link to the change record of this child issue.

I think we agreed here #3263618-10: Deprecate HAL module (and the comments above) to link to the documentation page instead of CRs.

Finally: Thanks _very_ much @larowlan and @quietone for taking this up and write it down. It prevents the good people trying to do the grind work from hitting The Concrete Wall if not absolutely unavoidable

Closing thoughts: By adding every sub-issue, however needed it might be, we add a potential blocker.
Not saying we should whack it all in one issue (Looking at you #3227033: Remove Quick Edit from core!), since that's too much, but (kinda) saying: Let's prevent shooting in own foot by making everything a sub-issue without it needing to be.

bbrala’s picture

bbrala
Location Netherlands
commented

The good thing about that though is that every step in the process is a managable piece of work. Which means there is at least a somewhat consistent set of small-ish milestones. This means you should be able to see progress more often and not end up with an issue with 6 discussions over 120 comments.

Basically, I think splitting in small steps is smart and prevents "death by a 1000 comments".

spokje’s picture

spokje
Primary language Dutch
commented

Basically, I think splitting in small steps is smart and prevents "death by a 1000 comments".

Fully agreed @bbrala, but it _could_ introduce "death by admin-overload of creating/managing 1000 sub-issues".

Anyway, besides the non-code references for the module-to-be-deprecated/removed deserving a separate sub-issue, as mentioned above and some minor nitpicks about lifecycle_links, I'm very happy with the defined stuff above and see no risk of any d.o.-queue-related deaths any time soon when we stick to it.

spokje’s picture

spokje
Primary language Dutch
commented

Some more do-this-before-that in a slack thread:

Deprecation + removal patches RTBC.
Stable release of the module.
Commit deprecation + removal patches within 24 hours of each other.
Open an issue against https://www.drupal.org/project/project_composer

quietone’s picture

quietone commented
Related issues: +#3264607: [policy, no patch] How to ensure migrations continue to work when modules are moved to contrib

Adding related migration issue that has more details of the migration steps which should be included in any documentation that is made from this discussion.

quietone’s picture

quietone commented
Issue summary: View changes

Updated the process based on last comments.

bbrala’s picture

bbrala
Location Netherlands
commented
Issue summary: View changes

I'd say first test composer install, then roll a stable release tbh. Updating is.

bbrala’s picture

bbrala
Location Netherlands
commented

When we agree on the process we can link ti relevant issues and such to make things easier.

catch’s picture

catch
he/him
Primary language English
commented

I added some initial documentation on the process as it stands so far to https://www.drupal.org/about/core/policies/core-change-policies/drupal-d... before seeing this thread. Haven't caught up here yet although there are steps in the issue summary I forgot to add to the docs.

quietone’s picture

quietone commented
Issue summary: View changes

Add removing from phpstan-baseline.neon. And a note about @todo statements.

@catch, thanks for adding this to the doc page.

Since the implementation details are so long, can that be a separate page?

quietone’s picture

quietone commented
Issue summary: View changes

Add migration details from #3264607: [policy, no patch] How to ensure migrations continue to work when modules are moved to contrib

quietone’s picture

quietone commented
Issue summary: View changes

Formatting

spokje’s picture

spokje
Primary language Dutch
commented

Wow, that's impressive work @quietone (and @larowlan)!

Would like some more eyes on this before RTBC, but I think you captured it all, at the very least this is a great fundament for a doc-page.

We might bump into some small fringe-issues whilst we deprecate/remove for D10, but this (again) at the very least covers all the basics.

catch’s picture

catch
he/him
Primary language English
commented

Since the implementation details are so long, can that be a separate page?

A separate page with a direct link sounds good to me. The main page could just define obsolete vs. deprecated and link?

bbrala’s picture

bbrala
Location Netherlands
commented

That would be best probably.

quietone’s picture

quietone commented
Status: Needs review » Reviewed & tested by the community

I created a new page, Module of theme removal process and updated Modules and themes on the deprecation policy page to point to former for the detailed steps. I don't think I lost any data in my changes but I am sure these will needed tweaks as module removals are worked on.

I think we have done everything that needs to be done here and and therefor setting to RTBC.

Is there more to do here?

spokje’s picture

spokje
Primary language Dutch
commented
Status: Reviewed & tested by the community » Fixed

As far as I can see this is (at the _very_ least) an excellent documentation of what we've learned so far whilst doing the core-deprecation/removal-dance.

As already said by @quietone:

but I am sure these will needed tweaks as module removals are worked on.

But now we have a base for the tweaks to be done on.

Marking this as fixed (with a big thanks to all involved, with bonus bow to @quietone and @larowlan)

bbrala’s picture

bbrala
Location Netherlands
commented

Changed 2 small typos. Think there was some theme mentions that should be module.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.