Rip out the update path

Created on 4 April 2023, about 1 year ago
Updated 20 April 2023, about 1 year ago

Problem/Motivation

While working on !821 in πŸ“Œ Remove all of our PHPStan `ignoreErrors` entries AND ignore PHPStan errors caused by upstream changes in core Fixed , I wondered: do we still need an update path in 3.0.x?

I'm not convinced we do. 3.0.x breaks backwards compatibility with a sledgehammer, and it can be increasingly tricky to maintain the update path (and its tests).

Proposed resolution

I propose we do the following:

  1. Done: git diff 3.0.x:automatic_updates.install 8.x-2.7:automatic_updates.install and git diff 3.0.x:automatic_updates.post_update.php 8.x-2.7:automatic_updates.post_update.php have no output, and diffing package_manager.install this way reveals no changes to the update hook.
  2. If it is, rip the update path, and its test, out of this module. We will need to implement hook_update_last_removed() and hook_removed_post_updates() to ensure that the core update system detects if you are not fully up to date.
  3. Done; see #7.

Remaining tasks

Do the stuff above, and commit if it all checks out.

πŸ“Œ Task
Status

Fixed

Version

3.0

Component

Code

Created by

πŸ‡ΊπŸ‡ΈUnited States phenaproxima Massachusetts

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

Comments & Activities

  • Issue created by @phenaproxima
  • πŸ‡ΊπŸ‡ΈUnited States phenaproxima Massachusetts
  • @phenaproxima opened merge request.
  • πŸ‡ΊπŸ‡ΈUnited States tedbow Ithaca, NY, USA
  • πŸ‡ΊπŸ‡ΈUnited States tedbow Ithaca, NY, USA
  • Status changed to Needs review about 1 year ago
  • πŸ‡ΊπŸ‡ΈUnited States tedbow Ithaca, NY, USA
  • Assigned to tedbow
  • πŸ‡ΊπŸ‡ΈUnited States phenaproxima Massachusetts

    Confirmed that update.php detects if you are on a too-old version of Automatic Updates.

    Assigning to @tedbow for final review.

  • Status changed to Needs work about 1 year ago
  • πŸ‡ΊπŸ‡ΈUnited States tedbow Ithaca, NY, USA

    I think `package_manager_update_last_removed` will also need to be removed from the core merge request..

    I can do this because will need to check this by running the conversion script.

  • Status changed to Needs review about 1 year ago
  • πŸ‡ΊπŸ‡ΈUnited States phenaproxima Massachusetts
  • Assigned to phenaproxima
  • Status changed to Needs work about 1 year ago
  • πŸ‡ΊπŸ‡ΈUnited States tedbow Ithaca, NY, USA

    When running the conversion script for the core merge this caused 2 problems 1 phpcs and 1 phpstan.

    I fixed the phpcs issue but still have this

    ----------------------------------------------------------------------------------------------------

    Running PHPStan on changed files.
    ------ ----------------------------------------------------------------------------------------------------------------------------------------------------------
    Line core/modules/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php
    ------ ----------------------------------------------------------------------------------------------------------------------------------------------------------
    94 Class Drupal\Tests\package_manager\Kernel\PackageManagerKernelTestBase has an uninitialized readonly property $fileSystem. Assign it in the constructor.
    ------ ----------------------------------------------------------------------------------------------------------------------------------------------------------

    [ERROR] Found 1 error

    Not actually use about this

  • Status changed to RTBC about 1 year ago
  • πŸ‡ΊπŸ‡ΈUnited States tedbow Ithaca, NY, USA

    If test pass then looks good

  • Status changed to Fixed about 1 year ago
  • πŸ‡ΊπŸ‡ΈUnited States phenaproxima Massachusetts
  • πŸ‡ΊπŸ‡ΈUnited States phenaproxima Massachusetts
  • Issue was unassigned.
  • Status changed to Fixed about 1 year ago
  • Automatically closed - issue fixed for 2 weeks with no activity.

Production build 0.69.0 2024