- Issue created by @quietone
- First commit to issue fork.
- last update
almost 2 years ago 29,559 pass - Status changed to Needs review
almost 2 years ago 2:06pm 27 June 2023 - ๐ง๐ทBrazil lucienchalom
I moved the deprecation error triger in classes:
3 core/lib/Drupal/Core/Asset/JsCollectionOptimizer.php
2 core/lib/Drupal/Core/Asset/CssCollectionOptimizer.phpand deleted the redundant trigger from
4 core/lib/Drupal/Core/Http/RequestStack.phpI am not really sure how to proceed with
5 core/modules/system/tests/modules/deprecation_test/src/Deprecation/DrupalStandardsListenerDeprecatedClass.php
6 core/modules/system/tests/modules/deprecation_test/src/Deprecation/FixtureDeprecatedClass.php
because they are "arbitrarily deprecated in order to test the deprecation error handling properties of DrupalStandardsListener."and
1 core/lib/Drupal/Component/Plugin/PluginHelper.php
does not accept a contruct.I tried to look for other classes deprecated but could not find any concrete, instantiated classes.
please help and review
- Status changed to RTBC
almost 2 years ago 3:28pm 27 June 2023 - ๐บ๐ธUnited States smustgrave
I think this is good but not sure the policy for moving existing deprecation triggers.
- last update
almost 2 years ago 29,567 pass - last update
almost 2 years ago 29,571 pass - last update
almost 2 years ago 29,801 pass - Status changed to Needs work
almost 2 years ago 7:31am 4 July 2023 - ๐ณ๐ฟNew Zealand quietone
@lucienchalom, thanks for working on this! This is a good start. For PluginHelper and RequestStack add a constructor (stated in policy) and then the test will need to be changed to instantiate the class. And the same for the test modules because we should be using the standard set in policy and well as testing that.
@smustgrave, There is no policy for moving deprecations. There is a policy for concrete classes and core needs to change to met the current standard.
The issue summary has a sample grep to find the instances that need to be changed. Has anyone worked to improve on that or verify that it is accurate?
- Status changed to Active
22 days ago 1:06am 25 April 2025 - ๐ณ๐ฟNew Zealand quietone
Rebased and updated the issue summary.
I think that this is suitable as a novice level issue.
- ๐ฎ๐ณIndia sanket.tale
sanket.tale โ made their first commit to this issueโs fork.
- ๐ฎ๐ณIndia sanket.tale
Hi Updated both Updater.php and I18nQueryTrait.php. Please review.
- ๐บ๐ธUnited States smustgrave
We are adding a trait to deprecate it? That doesnโt seem right,
Deprecations should probably get a CR too
- ๐ฎ๐ณIndia sanket.tale
Thank you for flagging this! After verifying that the I18nQueryTrait is not used anywhere, Iโve removed it entirely. Iโve also updated the existing Change Record to reflect this change. You can check it out here: Change Record - https://www.drupal.org/node/3439256 โ .
- ๐บ๐ธUnited States dww
This MR seems wrong.
Whatever changes were done in #6 are all gone.
#13 is also worrisome. "After verifying that the I18nQueryTrait is not used anywhere, Iโve removed it entirely." ๐ฌ That's not how this works. ๐ We mark something deprecated and *leave it where it is* so that contrib and custom code isn't immediately broken. That's the point of deprecation. So yes, it's unused in Core. But it needs to remain until D12 when we told people we'd remove it from its current location. I'm afraid of whatever edits were made to a live CR about this. Those probably need to be reverted, too. Although thankfully, https://www.drupal.org/node/3439256/revisions/view/13881188/13973995 โ doesn't look so bad (other than using markdown syntax which isn't yet supported on d.o CRs and docs pages).
I'm not sure this issue is really suitable for "novice" contributors.
- ๐บ๐ธUnited States dww
p.s. I'm unclear why
core/lib/Drupal/Core/Updater/Updater.php
is being touched, since that is in fact anabstract
class, so the existing deprecation is following the documented procedure. - ๐ณ๐ฟNew Zealand quietone
@dww, thanks for commenting.
This work has not followed the directions given in the issue summary. So, I started over with a new search
$ git grep -A3 '^namespace' | grep trigger core/lib/Drupal/Component/Utility/DeprecatedArray.php- * An array that triggers a deprecation warning when accessed. core/lib/Drupal/Core/FileTransfer/ChmodInterface.php-@trigger_error('The ' . __NAMESPACE__ . '\ChmodInterface is deprecated in drupal:11.2.0 and is removed from drupal:12.0.0. There is no replacement. Use composer to manage the code for your site. See https://www.drupal.org/node/3512364', E_USER_DEPRECATED); core/lib/Drupal/Core/Updater/Updater.php-@trigger_error('The ' . __NAMESPACE__ . '\Updater base class is deprecated in drupal:11.2.0 and is removed from drupal:12.0.0. There is no replacement. Use composer to manage the code for your site. See https://www.drupal.org/node/3512364', E_USER_DEPRECATED); core/lib/Drupal/Core/Updater/UpdaterInterface.php-@trigger_error('The ' . __NAMESPACE__ . '\UpdaterInterface is deprecated in drupal:11.2.0 and is removed from drupal:12.0.0. There is no replacement. Use composer to manage the code for your site. See https://www.drupal.org/node/3512364', E_USER_DEPRECATED); core/modules/system/tests/modules/deprecation_test/src/Deprecation/DrupalStandardsListenerDeprecatedClass.php-@trigger_error(__NAMESPACE__ . '\DrupalStandardsListenerDeprecatedClass is deprecated.', E_USER_DEPRECATED); core/modules/system/tests/modules/deprecation_test/src/Deprecation/FixtureDeprecatedClass.php-@trigger_error(__NAMESPACE__ . '\FixtureDeprecatedClass is deprecated.', E_USER_DEPRECATED);
The remaining deprecations just after the namespace are correct. ChmodInterface.php and UpdaterInterface.php are interfaces and Updater.php is an abstract class. Thus, they have implemented the deprecation policy correctly.
The test classes, however are not using the correct style, they need to be looked at to make sure that there is test coverage for the deprecation of concrete classes in the constructor.
I am removing the novice tag. Removed this issue from the change record, https://www.drupal.org/node/3439256 โ
Active because this is starting over.