Problem/Motivation

Follow up to #3591513: Deprecate the update_fetch_with_http_fallback setting

Actual removal happens here.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3613344

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

mstrelan created an issue. See original summary.

macsim made their first commit to this issue’s fork.

macsim opened merge request !16465

macsim’s picture

macsim commented
Status: Active » Needs review

My concern is that I had to remove testRealDeprecatedSettings and its data provider from SettingsTest. The test verifies that a deprecated setting with no replacement triggers a deprecation when present in settings.php. It requires at least one real entry in $deprecatedSettings to run.

update_fetch_with_http_fallback was the only such entry. With it gone, the provider returns nothing and the test cannot run.

The correct fix would be to keep the test and point it at another setting that is deprecated without replacement. No such setting appears to exist for the Drupal 12 transition. If one is added later, testRealDeprecatedSettings and providerTestRealDeprecatedSettings should be restored with the new entry as test data.

Another thing: PHPUnit failure is not related to the current changes.

mstrelan’s picture

mstrelan commented
Status: Needs review » Needs work

FWIW the test method used to have $this->markTestSkipped('No settings to test real deprecated settings with.');, which we could/should do now too.

Theoretically we could put a test value in \Drupal\Core\Site\Settings::$deprecatedSettings or manipulate that in the test with reflection. And now that I am writing this I see we already have \Drupal\Tests\Core\Site\SettingsTest::testFakeDeprecatedSettings.

I think the simplest path forward is to restore the test skipping. Ideally it would only be skipped if the property was empty, but since it's private we can't check that without reflection. That might be worth doing though, so next time a setting is deprecated we don't have to remember to unskip the test.

mstrelan’s picture

mstrelan commented
Status: Needs work » Needs review

Restored and skipped testRealDeprecatedSettings. Forget what I said in #5, I think that's too much trouble with the data provider.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community
Parent issue: » #3553503: [meta] Remove deprecated classes, methods, procedural functions and code paths outside of deprecated modules from Drupal 12

Seems like a good removal. And thanks for not removing that as it will be good reference for others later.

Tagging the parent issue.

quietone’s picture

quietone commented
Title: [12.x] Remove update_fetch_with_http_fallback » Remove update_fetch_with_http_fallback

  • catch committed 5a7489d2 on main 
    task: #3613344 Remove update_fetch_with_http_fallback

By: mstrelan
By:...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to main, thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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