Problem/Motivation

This is a follow-on from #3048498: [≈Nov. 11] Fix Drupal.Commenting.Deprecated coding standard where the automated patch fixed 653 of the 675 @deprecated messages reported under the Drupal.Commenting.Deprecated.IncorrectTextLayout sniff.

Remaining tasks

  1. Commit the first huge automated conversion chunk
  2. Follow-up with patches to fix the rest manually (Core 9.1 and 9.0)
  3. Enable the new Drupal.Commenting.Deprecated in phpcs.xml (Core 9.1 and 9.0)
  4. Fix the rest manually (Core 8.9)
  5. Enable the new Drupal.Commenting.Deprecated in phpcs.xml (Core 8.9)

Release note snippet

The new Drupal.Commenting.Deprecated rule has been enabled to ensure that @deprecated documentation follows a standard format with relevant information for upgrading the code. See the deprecation policy for documentation of the standard.

CommentFileSizeAuthor
#88 3094454-88.interdiff-83-to-88.txt3.94 KBjonathan1055
#88 3094454-88.patch21.43 KBjonathan1055
#83 interdiff-79-83.txt1.45 KBjungle
#83 3094454-83.patch23.52 KBjungle
#79 interdiff-73-79.txt2.05 KBjungle
#79 3094454-79.patch22.07 KBjungle
#76 interdiff-3094454-72-76.txt1.85 KBsja112
#76 3094454-76.patch22.01 KBsja112
#73 interdiff-70-72.txt637 bytesjungle
#73 3094454-72.patch20.89 KBjungle
#70 interdiff-68-70.txt10.67 KBjungle
#70 3094454-70.patch20.84 KBjungle
#68 3094454-68.fix_@deprecated_D89.patch20.47 KBsja112
#67 3094454-67.fix_@deprecated_D89.patch22.24 KBsja112
#64 interdiff-3094454-62-64.txt2.56 KBsja112
#64 3094454-64.fix_@deprecated_D89.patch20.47 KBsja112
#62 interdiff-3094454-52-62.txt15.75 KBsja112
#62 3094454-62.fix_@deprecated_D89.patch20.47 KBsja112
#52 3094454-52.fix_@deprecated_D89.patch19.32 KBjonathan1055
#51 3094454-51.fix_@deprecated_D89.patch21.09 KBjonathan1055
#45 3094454-45.fix_@deprecated_D89.patch21.41 KBjonathan1055
#23 3094454-23.patch1.25 KBlongwave
#22 3094454-22.fix_@deprecated_D9.patch3.24 KBjonathan1055
#20 3094454-20.fix_@deprecated_D9.patch2.36 KBjonathan1055
#18 3094454-18.show_all_@deprecated.patch2.51 KBjonathan1055
#17 3094454-17.show_all_@deprecated_and_@trigger_error.patch2.87 KBjonathan1055
#10 3094454-10.fix_@deprecated_text.patch21.29 KBjonathan1055
#9 3094454-9.show_@deprecated_layout_errors.patch2.99 KBjonathan1055
#8 3094454-8.fix_@deprecated_text.patch17.69 KBjonathan1055
#6 3094454-6.fix_@deprecated_text.patch18.88 KBjonathan1055
#5 3094454-5.interdiff-4-to-5.txt7.84 KBjonathan1055
#5 3094454-5.fix_@deprecated_text.patch21.74 KBjonathan1055
#4 3094454-4.fix_@deprecated_text.patch27.61 KBjonathan1055
#3 3094454-3.fix_@deprecated_text.patch21.55 KBjonathan1055
#2 3094454-2.show_@deprecated_layout_errors.patch10.73 KBjonathan1055
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jonathan1055 created an issue. See original summary.

jonathan1055’s picture

Title: Fix remaingin @Deprecated layout manually and enable the coding standard » Fix remaining @Deprecated layout manually and enable the coding standard
Status: Needs work » Needs review
FileSize
10.73 KB

First patch to show the remaining warnings that need to be fixed manually.

jonathan1055’s picture

That's the 22 that need fixing. Here's a patch with a few of them corrected, just to check that the mechanism is working as intended.

jonathan1055’s picture

Patch to fix the full 22 remaining messages.

To help with finding the version I used git show 55a4f988c1f which gave #2854830: Move rest/serialization module's "link manager" services to HAL module and git show bea23d7ce26 which gave #2949964: Add an EntityOwnerTrait to standardize the base field needed by EntityOwnerInterface

jonathan1055’s picture

Whilst making these manual changes, having the change record and/or issue link I thought I would add the missing @see links and missing extra-info. These will be required later when we fix Drupal.Commenting.Deprecated.MissingExtraInfo and Drupal.Commenting.Deprecated.MissingSeeTag but given that these lines are being changed in this patch anyway, it would save future effort if they were fixed now.

Source of info I used, in addition the the links in #4:
#2912169: Inject the argument resolver into HttpKernel::__construct
CR https://www.drupal.org/node/2959408

#2932462: Add EntityContextDefinition for the 80% use case
CR https://www.drupal.org/node/2976400

#2926508: Add DateTimeNormalizer+TimestampNormalizer, deprecate TimestampItemNormalizer: @DataType-level normalizers are reusable by JSON:API
CR https://www.drupal.org/node/2934779

jonathan1055’s picture

So that's all clean when adding the two new sniffs:

<rule ref="Drupal.Commenting.Deprecated.IncorrectTextLayout"/>
<rule ref="Drupal.Commenting.Deprecated.IncorrectTextLayoutFixable"/>

Here's the actual patch for review and commit, same as #5 but with no changes to drupalci.yml or phpcs.xml.dist

Status: Needs review » Needs work

The last submitted patch, 6: 3094454-6.fix_@deprecated_text.patch, failed testing. View results

jonathan1055’s picture

Status: Needs work » Needs review
FileSize
17.69 KB

All of the failures in #6 have the deprecation message

The "workflows.access_check.delete_state" service relies on the deprecated "Drupal\workflows\WorkflowDeleteAccessCheck" class. It should either be deprecated or its implementation upgraded.

It seeems that due to luck and/or a quirk of how the @deprecated tag was added to WorkflowDeleteAccessCheck that core tests should have been throwing this deprecation message all along. Fixing the standard has alerted us to this.

Patch #8 is the same as #6 but with the single change to WorkflowDeleteAccessCheck removed.

jonathan1055’s picture

#3009848: Fully deprecate WorkflowDeleteAccessCheck is done so now these last few @deprecated text layouts can be fixed. First, check to see the remaining problems.

jonathan1055’s picture

Patch #9 shows 22 @deprecated faults over 19 files. We had 22 before the WorkflowDeleteAccessCheck was fixed, so only one new bad @depreated basic layout was committed to core since November. That matches with the changes I had before + one new one. Patch #10 fixes those 22.

Gábor Hojtsy’s picture

Status: Needs review » Needs work

Woot, thanks!

Why do you need the host command?

We would need a patch without the drupalci.yml file changes to commit. I think we should commit to Drupal 9 for sure, but for consistency and easier deprecated API detection (eg. actionability), we should commit to Drupal 8.9 as well.

jonathan1055’s picture

The host command is to patch Coder because my final changes are not yet in a tagged release. I discovered early on in the analysis that lots of the @deprecated annotations in Core are correct (or could be automatically corrected) but do not have any 'extra info'. So one of the enhancements is to cater for this in a separate sniff so that the main text layout can pass. This was in place for all the work on #3048498: [≈Nov. 11] Fix Drupal.Commenting.Deprecated coding standard

So the 22 corrections could be committed now, but we cannot turn on the actual sniff until (a) patch #16 on #3057988-16: Automatically fixing @deprecated doc text has been committed to Coder, (b) Coder has new tagged release, and (c) we confirm that Core uses that new release.

Gábor Hojtsy’s picture

Can the coder fix be committed first, so we can commit the fixes for deprecation messages and the coder rule enablement at the same time? Or is that not on the horizon?

jonathan1055’s picture

Can the coder fix be committed first

It is ready now, as far as I am concerned, and I would be happy for that to be committed any time. Needs @klausi or @pfrenssen to review and commit, and ask them the timescale for the next Coder release.

jonathan1055’s picture

Just to update this thread, @klausi reviewed my work on https://github.com/pfrenssen/coder/pull/90 and I have addressed his points. Now waiting for re-review.

Gábor Hojtsy’s picture

@jonathan1055: Drupal 9.0.x does not have much @deprecated anymore, a handful are left for Drupal 10. It would be the very best time now to turn on the rules in Drupal 9 before more non-conformant ones are added :)

jonathan1055’s picture

Yes, that sounds good. Here is patch which will show all the @deprecated and @trigger_error coding standards messages, to see what we get on 9.0

jonathan1055’s picture

35 messages in total, but we have not been fixing the @trigger_error messages yet in 8.9. So here is just the @deprecated sniff.

Gábor Hojtsy’s picture

That only has two woot :)

core/includes/update.inc
line 18	The text '@deprecated in Drupal 8.8.4 and is removed from Drupal 9.0.0.' does not match the standard format: @deprecated in %deprecation-version% and is removed from %removal-version%. %extra-info%.

core/modules/system/tests/modules/deprecation_test/deprecation_test.module
14	Each @deprecated tag must have a @see tag immediately following it.
jonathan1055’s picture

Status: Needs review » Needs work

The last submitted patch, 20: 3094454-20.fix_@deprecated_D9.patch, failed testing. View results

jonathan1055’s picture

Status: Needs work » Needs review
FileSize
3.24 KB

Of course, the test for the expected deprecation message also has to be amended. I can't run local tests on D9 yet.

longwave’s picture

Rerolled, update_fix_compatibility() is gone so we can just enable the standard and fix the deprecation test module.

jonathan1055’s picture

Status: Needs review » Reviewed & tested by the community

Thanks longwave. Yes, to make sure of this, I've re-run patch #18 and it now shows not two but just the the one deprecation mesaage, in the test file which is fixed by patch #23

So #23 is RTBC. What other users need to OK this before it can be committed to 9.0.x?

longwave’s picture

We just need to wait for a core committer to look at this now it is RTBC.

xjm’s picture

Issue tags: +rc deadline

This has a (soft) RC deadline.

xjm’s picture

Issue tags: -rc deadline

(Wrong issue.)

xjm’s picture

Issue tags: +rc deadline

Although enabling new rules also typically has an RC deadline, so adding back the tag. :)

xjm’s picture

Title: Fix remaining @Deprecated layout manually and enable the coding standard » Fix remaining @deprecated (missing a change record) manually and enable the coding standard

  • xjm committed bd64783 on 9.1.x
    Issue #3094454 by jonathan1055, longwave: Fix remaining @deprecated (...

  • xjm committed 5230377 on 9.0.x
    Issue #3094454 by jonathan1055, longwave: Fix remaining @deprecated (...
xjm’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 9.1.x and 9.0.x. Thanks! It's great to finally see the standard adopted and will make the D10 upgrade even easier.

Normally we would also backport the new coding standard to 8.9.x, but in this case there are probably hundreds of issues missing change record links in 8.9 and 8.8, so we can't yet. There's still a meta issue open to add CRs to those old deprecations, but this will ensure the standard is always followed going forward.

Thanks!

xjm’s picture

Issue tags: +9.0.0 release notes

We list newly enabled coding standards in the release notes, also.

jonathan1055’s picture

Version: 9.0.x-dev » 8.9.x-dev

Thanks @xjm for committing to 9.0 and 9.1.

Normally we would also backport the new coding standard to 8.9.x, but in this case there are probably hundreds of issues missing change record links in 8.9 and 8.8, so we can't yet.

Well, there were 675 in 8.9.x but I managed to fix 653 of these basic layout problems automatically in #3048498: [≈Nov. 11] Fix Drupal.Commenting.Deprecated coding standard. There are now only 22 as per comment #9 above. This issue started at 8.9 but Gábor moved it on to 9.0 to get the standard in first. I will re-check/re-roll that patch and see where we are in 8.9

xjm’s picture

Status: Fixed » Patch (to be ported)

PTBP then?

jonathan1055’s picture

Version: 8.9.x-dev » 9.0.x-dev
Status: Patch (to be ported) » Fixed

Yes. But was I too fast in changing the version? Should it remain at 9.0 for the Change Record and Release Notes. I can still work on the patch anyway.

jonathan1055’s picture

Version: 9.0.x-dev » 8.9.x-dev
Status: Fixed » Patch (to be ported)

The patch will be bigger for 8.9. Going to work on this. Did not intend to set the status back to fixed in #36.

xjm’s picture

Issue tags: +Needs release note

I don't think we need a CR, although yeah a release note would be good since we need to say something.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

alexpott’s picture

Whilst committing another patch I ran into

FILE: ...ers/alex/dev/drupal/core/lib/Drupal/Core/Database/Connection.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 1536 | ERROR | The text '@deprecated in drupal:9.1.0 and is removed
      |       | in drupal:10.0.0. All database drivers must support
      |       | transactions.' does not match the standard format:
      |       | @deprecated in %deprecation-version% and is removed
      |       | from %removal-version%. %extra-info%.
----------------------------------------------------------------------

#23 should have been done against 9.1.x - but this is the only instance so I've hotfixed it as changing in to from feels very minor.

  • alexpott committed c285e2b on 9.1.x
    Issue #3094454 follow-up by alexpott: Fix remaining @deprecated (missing...
pameeela’s picture

@jonathan1055 we're preparing the release notes, can you update the IS with a release note snippet? Just a brief summary of the change with a link to additional documentation if needed.

xjm’s picture

Title: Fix remaining @deprecated (missing a change record) manually and enable the coding standard » [backport] Fix remaining @deprecated (missing a change record) manually and enable the coding standard
Version: 9.1.x-dev » 8.9.x-dev
jonathan1055’s picture

Issue summary: View changes

Hi @pameela,
The issue where the standards were agreed is #3024461: Adopt consistent deprecation format for core and contrib deprecation messages. Even though that issue summary says "proposed" standard, it has been agreed in that thread, and completed in Coder. See also https://www.drupal.org/core/deprecation

I have updated this IS with the standard and example. and split the tasks over the D9 and D8.9 version for clarity. Hopefully that is what you wanted?

jonathan1055’s picture

Status: Patch (to be ported) » Needs review
FileSize
21.41 KB

Now we are back with Core 8.9. This new patch #45 is essentially the same as #10 (which still applies). However, now that Coder 8.3.9 has been released we can ue that instead of applying the patch to Coder 8.3.8. Hopefully this change to drupalci.yml will work:

     validate_codebase:
+      container_command:
+        # Core currently uses Coder 8.3.8 but we need the changes now in 8.3.9
+        commands:
+          - "cd ${SOURCE_DIR} && sudo -u www-data composer require drupal/coder:^8.3.9"
       phplint:
longwave’s picture

Build failure is:

16:47:29 cd core && sudo -u www-data /var/www/html/vendor/bin/phpcs --report-full=/var/lib/drupalci/workdir/phpcs/codesniffer_results.txt --report-checkstyle=/var/lib/drupalci/workdir/phpcs/checkstyle.xml --report-diff=/var/lib/drupalci/workdir/phpcs/codesniffer_fixes.patch /var/www/html/core
16:47:29 ERROR: Referenced sniff "Drupal.Arrays.Array" does not exist
16:47:29 
16:47:29 Run "phpcs --help" for usage information
jungle’s picture

BTW, the failure in #45 might be related to the bug that coder 8.3.9 got installed to module/contribute.

jungle’s picture

jonathan1055’s picture

@jungle how did you see that coder was installed at module/contribute ?

So we have

Package operations: 0 installs, 1 update, 0 removals
00:03:35.011   - Removing drupal/coder (8.3.8)

and

- Installing drupal/coder (8.3.9):

and we also have in the phpcs output

Host commands: /var/www/html/vendor/bin/phpcs -i
Return code: 0
Output: The installed coding standards are Zend, Squiz, PSR2, PSR12, PSR1, PEAR and MySource

which does imply that the Drupal and DrupalPractice standards are not available.

I added cd ${SOURCE_DIR} but was that not the right place to attempt to upgrade coder?

jungle’s picture

Status: Needs review » Needs work

Just guessing. I encountered that error before due to the reason i mentioned. Anyway, coder 8.3.9 was in, let's revert the DrupalCI customization?

jonathan1055’s picture

Status: Needs work » Needs review
FileSize
21.09 KB

Yes, Coder 8.3.9 release was requested to allow my changes to be available for these issues. Great that @klausi and @alexpott committed those so quickly.

Here is patch #41 but without the attempt in drupalci to upgrade Coder.

jonathan1055’s picture

All clean, so this patch #52 should be the actual final patch for committing to core 8.9. This will enable the Drupal.Commenting.Deprecated rule and use sniff Drupal.Commenting.Deprecated.IncorrectTextLayout but exclude the other 5 sniffs in this standard.

xjm’s picture

Thanks @jonathan1055. We don't need to re-document the coding standard in the release note; we just need to say that the standard is now enabled for the core ruleset. I think we could just say that deprecation messages must now follow the documented format and link the handbook page for the example.

You can check past minor release notes for an example of the release notes about enabled coding standards rules. (I don't recall offhand which might have an example, but check 8.8.0, 8.7.0, etc.).

xjm’s picture

Issue summary: View changes
Issue tags: -Needs release note

Updated with a release note that explains the purpose of the rule and links the deprecation documentation.

jungle’s picture

Status: Needs review » Reviewed & tested by the community

#52 LGTM, no CS violations, thanks!

xjm’s picture

Status: Reviewed & tested by the community » Needs work

Thanks -- mostly just some small formatting feedback.

  1. +++ b/core/lib/Drupal/Component/Utility/SafeMarkup.php
    @@ -9,9 +9,10 @@
    + * @deprecated in drupal:8.0.0 and is removed from drupal:9.0.0.
    + *   Use the appropriate @link sanitization sanitization functions @endlink or
    
    +++ b/core/lib/Drupal/Core/Block/Annotation/Block.php
    @@ -45,9 +45,11 @@ class Block extends Plugin {
    +   * @deprecated in drupal:8.7.0 and is removed from drupal:9.0.0.
    +   *   Providing context definitions via the "context" key is deprecated. Use
    
    +++ b/core/lib/Drupal/Core/Condition/Annotation/Condition.php
    @@ -55,9 +55,11 @@ class Condition extends Plugin {
    +   * @deprecated in drupal:8.7.0 and is removed from drupal:9.0.0.
    +   *   Providing context definitions via the "context" key is deprecated. Use
    
    +++ b/core/lib/Drupal/Core/Controller/FormController.php
    @@ -27,9 +27,9 @@
    +   * @deprecated in drupal:8.6.0 and is removed from drupal:9.0.0.
    +   *   This property is only assigned when the 'controller_resolver' service
    
    +++ b/core/lib/Drupal/Core/Entity/EntityManager.php
    @@ -788,9 +788,8 @@ public function getLastInstalledDefinition($entity_type_id) {
    +   * @deprecated in drupal:8.0.0 and is removed from drupal:9.0.0.
    +   *   Use \Drupal\Core\Entity\EntityTypeManagerInterface::useCaches() and/or
    
    +++ b/core/lib/Drupal/Core/Menu/LocalActionManager.php
    @@ -60,7 +60,7 @@ class LocalActionManager extends DefaultPluginManager implements LocalActionMana
    +   * @deprecated in drupal:8.6.0 and is removed from drupal:9.0.0.
        *   Using the 'controller_resolver' service as the first argument is
    
    +++ b/core/lib/Drupal/Core/Menu/LocalTaskManager.php
    @@ -63,7 +63,7 @@ class LocalTaskManager extends DefaultPluginManager implements LocalTaskManagerI
    +   * @deprecated in drupal:8.6.0 and is removed from drupal:9.0.0.
        *   Using the 'controller_resolver' service as the first argument is
    
    +++ b/core/lib/Drupal/Core/Path/AliasStorage.php
    @@ -18,10 +18,9 @@
    + * @deprecated in drupal:8.8.0 and is removed from drupal:9.0.0.
    + *   Use the "path_alias.repository" service instead, or the entity storage
    
    +++ b/core/lib/Drupal/Core/Path/AliasStorageInterface.php
    @@ -7,10 +7,9 @@
    + * @deprecated in drupal:8.8.0 and is removed from drupal:9.0.0.
    + *   Use the "path_alias.repository" service instead, or the entity storage
    
    +++ b/core/lib/Drupal/Core/Plugin/Context/ContextDefinition.php
    @@ -78,24 +78,23 @@ class ContextDefinition implements ContextDefinitionInterface {
    +   * @deprecated in drupal:8.6.0 and is removed from drupal:9.0.0.
        *   Constructing a context definition for an entity type (i.e., the data type
    
    @@ -432,10 +431,11 @@ public function __sleep() {
    +   * @deprecated in drupal:8.6.0 and is removed from drupal:9.0.0.
    +   *   This method should be kept private so that it is only accessible to this
    
    +++ b/core/modules/content_moderation/src/Entity/ContentModerationState.php
    @@ -156,8 +156,10 @@ public static function loadFromModeratedEntity(EntityInterface $entity) {
    +   * @deprecated in drupal:8.6.0 and is removed from drupal:9.0.0.
    +   *   Use Drupal\user\EntityOwnerTrait::getDefaultEntityOwner() instead.
    
    +++ b/core/modules/hal/tests/modules/hal_test/hal_test.module
    @@ -26,7 +26,9 @@ function hal_test_hal_relation_uri_alter(&$uri, $context = []) {
    + * @deprecated in drupal:8.3.0 and is removed from drupal:9.0.0.
    + *   Implement hook_hal_type_uri_alter() instead.
    
    @@ -39,7 +41,9 @@ function hal_test_rest_type_uri_alter(&$uri, $context = []) {
    + * @deprecated in drupal:8.3.0 and is removed from drupal:9.0.0.
    + *   Implement hook_hal_relation_uri_alter() instead.
    
    +++ b/core/modules/media/src/Entity/Media.php
    @@ -512,8 +512,10 @@ public static function baseFieldDefinitions(EntityTypeInterface $entity_type) {
    +   * @deprecated in drupal:8.6.0 and is removed from drupal:9.0.0.
    +   *   Use Drupal\user\EntityOwnerTrait::getDefaultEntityOwner() instead.
    
    +++ b/core/modules/rest/src/Plugin/Type/ResourcePluginManager.php
    @@ -37,7 +37,7 @@ public function __construct(\Traversable $namespaces, CacheBackendInterface $cac
    +   * @deprecated in drupal:8.2.0 and is removed from drupal:9.0.0.
        *   Use Drupal\rest\Plugin\Type\ResourcePluginManager::createInstance()
    
    +++ b/core/modules/serialization/src/Normalizer/TimeStampItemNormalizerTrait.php
    @@ -9,7 +9,10 @@
    + * @deprecated in drupal:8.7.0 and is removed from drupal:9.0.0.
    + *   Use \Drupal\serialization\Normalizer\TimestampNormalizer instead.
    
    +++ b/core/modules/workspaces/src/Entity/Workspace.php
    @@ -196,8 +196,10 @@ public static function postDelete(EntityStorageInterface $storage, array $entiti
    +   * @deprecated in drupal:8.6.0 and is removed from drupal:9.0.0.
    +   *   Use Drupal\user\EntityOwnerTrait::getDefaultEntityOwner() instead.
    

    All of these are wrapping too early on the second line, I think.

  2. +++ b/core/lib/Drupal/Component/Utility/SafeMarkup.php
    @@ -56,7 +57,7 @@ public static function isSafe($string, $strategy = 'html') {
    -   * @deprecated Will be removed before Drupal 9.0.0. Rely on Twig's
    +   * @deprecated in drupal:8.0.0 and is removed from drupal:9.0.0. Rely on Twig's
    

    Over 80 chars.

  3. +++ b/core/lib/Drupal/Core/Controller/ArgumentResolver/RawParameterValueResolver.php
    @@ -11,8 +11,8 @@
    - * @deprecated in Drupal 8.8.1 and will be removed before Drupal 9.0.0. There is
    - *   no replacement.
    + * @deprecated in drupal:8.8.1 and is removed from drupal:9.0.0.
    + *   There is no replacement.
    

    Was there also never a change record? If so, should we fix that?

  4. +++ b/core/modules/hal/tests/modules/hal_test/hal_test.module
    @@ -26,7 +26,9 @@ function hal_test_hal_relation_uri_alter(&$uri, $context = []) {
    + *   Kept only for BC test coverage, see \Drupal\Tests\hal\Kernel\HalLinkManagerTest::testGetTypeUri().
    
    @@ -39,7 +41,9 @@ function hal_test_rest_type_uri_alter(&$uri, $context = []) {
    + *   Kept only for BC test coverage, see \Drupal\Tests\hal\Kernel\HalLinkManagerTest::testGetRelationUri().
    

    Also over 80 chars. (It was before also, but that doesn't mean we should persist with that when re-wrapping.)

jyotimishra-developer’s picture

Assigned: Unassigned » jyotimishra-developer
jonathan1055’s picture

Assigned: jyotimishra-developer » Unassigned

Hi jyotimishra123,
I am 3/4 way through fixing this. How much have you done yet? Sorry I did not assign it to me, as I had been the only one working on this patch, so just assumed no one else would do it.

[edit: Argh, did not mean to unassign, it was stale form data. If you want to do the work, let me know here]

sja112’s picture

Assigned: Unassigned » sja112

@jonathan1055, working on this.

jonathan1055’s picture

sja112? OK, even though I have got it nearly all done. Just for info, and to save you some effor, for (2) you can have

-   * @deprecated in drupal:8.0.0 and is removed from drupal:9.0.0. Rely on Twig's
+   * @deprecated in drupal:8.0.0 and is removed from drupal:9.0.0. Twig has an

because otherwise the next line will extent beyond 80 but you can't fix that as the @link @endlink has to be in the same line. You can update the trigger_error text to match this change, or it can be done separately, as there is another issue for that.

And also don't fix (3) as that is not in scope - the sniff Drupal.Commenting.Deprecated.DeprecatedMissingSeeTag will be covered in a separate issue, we are only fixing the main Drupal.Commenting.Deprecated.IncorrectTextLayout sniff here and enabling the rule.

sja112’s picture

jonathan1055, Thanks for the info. I will update the patch accordingly.

sja112’s picture

Assigned: sja112 » Unassigned
Status: Needs work » Needs review
FileSize
20.47 KB
15.75 KB

@xjm, Updated the patch as per the review comments.

As per comment #60,

don't fix (3) as that is not in scope - the sniff Drupal.Commenting.Deprecated.DeprecatedMissingSeeTag will be covered in a separate issue, we are only fixing the main Drupal.Commenting.Deprecated.IncorrectTextLayout sniff here and enabling the rule.

haven't worked on #56.3

Thanks!

jungle’s picture

Status: Needs review » Needs work

Thanks @sja112!

+++ b/core/lib/Drupal/Core/Block/Annotation/Block.php
@@ -45,9 +45,11 @@ class Block extends Plugin {
+   * @deprecated in drupal:8.7.0 and is removed from drupal:9.0.0. Providing context

+++ b/core/lib/Drupal/Core/Condition/Annotation/Condition.php
@@ -55,9 +55,11 @@ class Condition extends Plugin {
+   * @deprecated in drupal:8.7.0 and is removed from drupal:9.0.0. Providing context

+++ b/core/lib/Drupal/Core/Plugin/Context/ContextDefinition.php
@@ -78,24 +78,23 @@ class ContextDefinition implements ContextDefinitionInterface {
+   * @deprecated in drupal:8.6.0 and is removed from drupal:9.0.0. Constructing a

Exceed 80 chars.

sja112’s picture

Status: Needs work » Needs review
FileSize
20.47 KB
2.56 KB

@jungle, Thanks for the review.

I have updated the patch as per your review comment.

jungle’s picture

Status: Needs review » Needs work

@sja112, thank for your quick response!

+++ b/core/lib/Drupal/Core/Plugin/Context/ContextDefinition.php
@@ -80,8 +80,8 @@ class ContextDefinition implements ContextDefinitionInterface {
-   * @deprecated in drupal:8.6.0 and is removed from drupal:9.0.0. Constructing a
-   *   context definition for an entity type (i.e., the data type
...
+   *   a context definition for an entity type (i.e., the data type
    *   begins with 'entity:') is deprecated. Instead, use the static factory
    *   methods of EntityContextDefinition to create context definitions for
    *   entity types, or the static ::create() method of this class for any other

The lines after a context definition for an entity type (i.e., the data type should be adjusted accordingly. for instance, moving begin or begin with to the previous line, as long as it does not exceed 80 chars.

Tip: you can cancel the CI run on the CI job page, for instance: https://www.drupal.org/pift-ci-job/1692122

jonathan1055’s picture

Status: Needs work » Needs review

Second tip, as we are only running coding standards, you can use the change to drupalci.yml in the top of my patch 51 https://www.drupal.org/files/issues/2020-05-13/3094454-51.fix_%40depreca... so that the run ends very quickly, saving resource. Then when all the changes are agreed, remove that and run one final time.

[edit: stale form data again! did not mean to set to needs review, but it will be needs review soon, so no point changing it back now]

sja112’s picture

As per #65 and #66, I have updated the patch.

sja112’s picture

In #67 haven't received any error related to character count. Reverting the changes and adding the final patch for review.

Thanks once again @jonathan1055 for help.

jungle’s picture

Assigned: Unassigned » jungle
Status: Needs review » Needs work
+++ b/core/lib/Drupal/Core/Controller/FormController.php
@@ -27,9 +27,9 @@
+   * @deprecated in drupal:8.6.0 and is removed from drupal:9.0.0. This property
+   *   is only assigned when the 'controller_resolver' service is
+   *   used as the first parameter to FormController::__construct().

+++ b/core/lib/Drupal/Core/Menu/LocalActionManager.php
@@ -60,9 +60,9 @@ class LocalActionManager extends DefaultPluginManager implements LocalActionMana
+   *   'controller_resolver' service as the first argument is deprecated,
+   *   use the 'http_kernel.controller.argument_resolver' instead.

+++ b/core/lib/Drupal/Core/Menu/LocalTaskManager.php
@@ -63,9 +63,9 @@ class LocalTaskManager extends DefaultPluginManager implements LocalTaskManagerI
+   *   'controller_resolver' service as the first argument is deprecated,
+   *   use the 'http_kernel.controller.argument_resolver' instead.

+++ b/core/lib/Drupal/Core/Path/AliasStorageInterface.php
@@ -7,10 +7,9 @@
+ *   "path_alias.repository" service instead, or the entity storage
+ *   handler for the "path_alias" entity type for CRUD methods.

+++ b/core/lib/Drupal/Core/Plugin/Context/ContextDefinition.php
@@ -78,24 +78,23 @@ class ContextDefinition implements ContextDefinitionInterface {
+   *   of EntityContextDefinition to create context definitions for
+   *   entity types, or the static ::create() method of this class for any other

@@ -432,10 +431,11 @@ public function __sleep() {
+   *   should be kept private so that it is only accessible to this
+   *   class for backwards compatibility reasons.

More needs adjusting. Well, assign to myself.

jungle’s picture

Assigned: jungle » Unassigned
Status: Needs work » Needs review
FileSize
20.84 KB
10.67 KB

Addressing #69 and other small changes, details coming in #71

jungle’s picture

  1. +++ b/core/lib/Drupal/Component/Utility/SafeMarkup.php
    @@ -10,9 +10,9 @@
    - *   can can be themed, escaped, and altered properly.
    ...
    + *   theme_render theme and render systems @endlink so that the output can be
    

    Removed redundant "can"

  2. +++ b/core/lib/Drupal/Core/Entity/EntityManager.php
    @@ -788,9 +788,9 @@ public function getLastInstalledDefinition($entity_type_id) {
    -   *   Drupal\Core\Entity\EntityFieldManagerInterface::useCaches() instead.
    ...
    +   *   \Drupal\Core\Entity\EntityFieldManagerInterface::useCaches() instead.
    

    Added a prefix \

Changes in #70

jonathan1055’s picture

In #67 haven't received any error related to character count.

The actual sniff for character count is ignored for drupal core as there are too many files that fail this rule.

jungle’s picture

FileSize
20.89 KB
637 bytes

> #56.3 Was there also never a change record? If so, should we fix that?

No change record. Found that #3098814: Class 'Drupal\Core\Controller\ArgumentResolver\RawParameterValueResolver' not found during update to 8.8.0 introduced it.

The coming patch added a @see. So #70 or this one. Up to you :)

No testing triggered on purpose.

jungle’s picture

In addition to #72, as the NW request #56 was from a committer -- @xjm, so to me, even though this kind of changes are unnecessary, I am intending to address it. Similar message was delivered to me by @alexpott as committer, somewhere in the issue queue i could not remember.

jonathan1055’s picture

Status: Needs review » Needs work

Thanks @jungle and @sja112.

patch #72 added a @see. So #70 or this one. Up to you :)

Well, I was just trying to not let the scope creep, but as you've done the work and actually found a link, let's keep it.

Two things to query in patch #72
1.

--- a/core/lib/Drupal/Component/Utility/SafeMarkup.php
+++ b/core/lib/Drupal/Component/Utility/SafeMarkup.php
@@ -9,9 +9,10 @@
 /**
  * Contains deprecated functionality related to sanitization of markup.
  *
- * @deprecated Will be removed before Drupal 9.0.0. Use the appropriate
- *   @link sanitization sanitization functions @endlink or the @link theme_render theme and render systems @endlink
- *   so that the output can can be themed, escaped, and altered properly.
+ * @deprecated in drupal:8.0.0 and is removed from drupal:9.0.0. Use the
+ *   appropriate @link sanitization sanitization functions @endlink or the @link
+ *   theme_render theme and render systems @endlink so that the output can be
+ *   themed, escaped, and altered properly.

Is this really allowed? Can that @link be rendered in IDEs when it spans over a new line to the @endlink. Maybe it is OK?

2.
My suggested change in core/lib/Drupal/Component/Utility/SafeMarkup.php is good

-   * @deprecated Will be removed before Drupal 9.0.0. Rely on Twig's
+   * @deprecated in drupal:8.0.0 and is removed from drupal:9.0.0. Twig has an

However, to make it easier for someone in the future, we might as well make the same change in the @trigger_error text, as the standard is to have the same wording. It needs the following change:

-    @trigger_error('SafeMarkup::checkPlain() is scheduled for removal in Drupal 9.0.0. Rely on Twig\'s auto-escaping feature
+    @trigger_error('SafeMarkup::checkPlain() is deprecated in drupal:8.0.0 and is removed from drupal:9.0.0. Twig has an auto-escaping feature

There will be other @trigger_error messages which fail the standard, but those are for a separate issue. This one should be fixed here because we have actually changed the message wording very slightly to assist spanning and @link.

sja112’s picture

@jonathan1055,

Thanks for your input.

As per your comment #75.2 I have updated the patch.

Also includes fix for reoccurrence of word sanitizationtwice.

--- a/core/lib/Drupal/Component/Utility/SafeMarkup.php
+++ b/core/lib/Drupal/Component/Utility/SafeMarkup.php
@@ -9,9 +9,10 @@
 /**
  * Contains deprecated functionality related to sanitization of markup.
  *
- * @deprecated Will be removed before Drupal 9.0.0. Use the appropriate
- *   @link sanitization sanitization functions @endlink or the @link theme_render theme and render systems @endlink
- *   so that the output can can be themed, escaped, and altered properly.
+ * @deprecated in drupal:8.0.0 and is removed from drupal:9.0.0. Use the
+ *   appropriate @link sanitization sanitization functions @endlink or the @link
+ *   theme_render theme and render systems @endlink so that the output can be
+ *   themed, escaped, and altered properly.
sja112’s picture

Status: Needs work » Needs review
jonathan1055’s picture

Status: Needs review » Needs work

Also includes fix for reoccurrence of word 'sanitization' twice.

No, that's wrong. The word is needed twice. It is inside the @link, the first is the url, then the remaining is the text. See api-documentation-and-comment-standards#link

Also, as suggested in #75.2 if that line has to be changed it should be fixed for @trigger_error standard too, to save later work. I gave the actual text for that "SafeMarkup::checkPlain() is deprecated in drupal:8.0.0 and is removed from drupal:9.0.0." in that snippet.

jungle’s picture

Status: Needs work » Needs review
FileSize
22.07 KB
2.05 KB

Addressing #75, @jonathan1055, thanks for your review!

Is this really allowed? Can that @link be rendered in IDEs when it spans over a new line to the @endlink. Maybe it is OK?

Searched code in core, found no example of @link ... @endlink spanning more than one line. so adjusted again to make it one-line.

The last submitted patch, 76: 3094454-76.patch, failed testing. View results

jonathan1055’s picture

OK so if we fix that message, we also need to fix the test.

-%A  SafeMarkup::checkPlain() is scheduled for removal in Drupal 9.0.0. Rely on Twig's auto-escaping feature, or use the @link theme_render #plain_text @endlink key when constructing a render array that contains plain text in order to use the renderer's auto-escaping feature. If neither of these are possible, \Drupal\Component\Utility\Html::escape() can be used in places where explicit escaping is needed. See https://www.drupal.org/node/2549395.
+  SafeMarkup::checkPlain() is scheduled for removal in Drupal 9.0.0. Twig has an auto-escaping feature, or use the @link theme_render #plain_text @endlink key when constructing a render array that contains plain text in order to use the renderer's auto-escaping feature. If neither of these are possible, \Drupal\Component\Utility\Html::escape() can be used in places where explicit escaping is needed. See https://www.drupal.org/node/2549395.

Or do neither and leave it for the other issue.

Status: Needs review » Needs work

The last submitted patch, 79: 3094454-79.patch, failed testing. View results

jungle’s picture

Status: Needs work » Needs review
FileSize
23.52 KB
1.45 KB

Fixing the test.

jonathan1055’s picture

Status: Needs review » Reviewed & tested by the community

Nice work @jungle (and @sja112 before).
Thanks for adjusting the expected deprecation in the test, better to do that here as we are looking at these files and will save someone's effort in the future.

found no example of @link ... @endlink spanning more than one line.

That's right, and this also explicitly stated in api-documentation-and-comment-standards#link

Everthing in #56, #63, #65, #69, #71, #75, #78 and #81 are fixed so marking patch #83 as RTBC to allow core reviewer to follow up.

xjm’s picture

Status: Reviewed & tested by the community » Needs review
+++ b/core/lib/Drupal/Component/Utility/SafeMarkup.php
@@ -67,7 +68,7 @@ public static function isSafe($string, $strategy = 'html') {
-    @trigger_error('SafeMarkup::checkPlain() is scheduled for removal in Drupal 9.0.0. Rely on Twig\'s auto-escaping feature, or use the @link theme_render #plain_text @endlink key when constructing a render array that contains plain text in order to use the renderer\'s auto-escaping feature. If neither of these are possible, \Drupal\Component\Utility\Html::escape() can be used in places where explicit escaping is needed. See https://www.drupal.org/node/2549395.', E_USER_DEPRECATED);
+    @trigger_error('SafeMarkup::checkPlain() is deprecated in drupal:8.0.0 and is removed from drupal:9.0.0. Twig has an auto-escaping feature, or use the @link theme_render #plain_text @endlink key when constructing a render array that contains plain text in order to use the renderer\'s auto-escaping feature. If neither of these are possible, \Drupal\Component\Utility\Html::escape() can be used in places where explicit escaping is needed. See https://www.drupal.org/node/2549395.', E_USER_DEPRECATED);

🤔 Why are we changing the grammar here? The sentence is no longer parallel.

Is this because the escaped single quote is causing a false positive on a rule violation? If so, could we just convert the string to double quotes instead of changing the grammar?

jonathan1055’s picture

The sentence is no longer parallel.

The @deprecated change was

@@ -56,7 +57,7 @@ public static function isSafe($string, $strategy = 'html') {
    * @return \Drupal\Component\Render\HtmlEscapedText
    *   An HtmlEscapedText object that escapes when rendered to string.
    *
-   * @deprecated Will be removed before Drupal 9.0.0. Rely on Twig's
+   * @deprecated in drupal:8.0.0 and is removed from drupal:9.0.0. Twig has an
    *   auto-escaping feature, or use the @link theme_render #plain_text @endlink
    *   key when constructing a render array that contains plain text in order to
    *   use the renderer's auto-escaping feature. If neither of these are

which meant we also needed to keep the @trigger_error text matching it. It was your line-length > 80 query on this line which meant we had to change the @deprecated first line. But if it wrapped instead of shortening then the next line @link ... @endlink would extend beyond 80. However, I have now learned that @link .. @endlink syntax is allowed to extend beyond 80, so yes this grammar change could be reverted and the wrapping adjusted instead.

jonathan1055’s picture

Assigned: Unassigned » jonathan1055

Assigning.
I will re-do that change and revert the @trigger_error change

jonathan1055’s picture

FileSize
21.43 KB
3.94 KB

The @link ... @endlink is only allowed to extend beyond 80 when the entire link is bigger than 80. Having it extend beyond 80 when it can be moved to the next line is what the standard requires, hence the re-wrapping of the whole paragraph.

sja112’s picture

Assigned: jonathan1055 » Unassigned
Status: Needs review » Reviewed & tested by the community

@jonathan1055,

Thanks for the updated patch.

The @trigger_error, comment and test case changes are reverted.

#85 is addressed so marking patch #88 as RTBC.

xjm’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs followup
+++ b/core/lib/Drupal/Component/Utility/SafeMarkup.php
@@ -57,18 +57,19 @@ public static function isSafe($string, $strategy = 'html') {
-    @trigger_error('SafeMarkup::checkPlain() is deprecated in drupal:8.0.0 and is removed from drupal:9.0.0. Twig has an auto-escaping feature, or use the @link theme_render #plain_text @endlink key when constructing a render array that contains plain text in order to use the renderer\'s auto-escaping feature. If neither of these are possible, \Drupal\Component\Utility\Html::escape() can be used in places where explicit escaping is needed. See https://www.drupal.org/node/2549395.', E_USER_DEPRECATED);
+    @trigger_error('SafeMarkup::checkPlain() is scheduled for removal in Drupal 9.0.0. Rely on Twig\'s auto-escaping feature, or use the @link theme_render #plain_text @endlink key when constructing a render array that contains plain text in order to use the renderer\'s auto-escaping feature. If neither of these are possible, \Drupal\Component\Utility\Html::escape() can be used in places where explicit escaping is needed. See https://www.drupal.org/node/2549395.', E_USER_DEPRECATED);

Oops, sorry for not being clear. We can keep the standard: ...in drupal:8.0.0 and is removed from drupal:9.0.0. It's just more grammatically correct to say "Rely on Twig's auto-escaping feature, or use..." because then both parts start with an imperative verb.

There's also an interesting problem with the @link/@endlink stuff. On the SafeMarkup API documentation they link the correct documentation topics; however, they won't work in the @trigger_error() and so developers who find the message that way will have no information about what they're supposed to do.

But! I realized now all of that is out of scope, because this issue is just fixing the @deprecated, not @trigger_error(). We can file a followup to fix checkPlain()'s messages somehow.

Meanwhile, removing the rest.

jonathan1055’s picture

Hi xjm, thanks for the explanation and review.

We can keep the standard: ...in drupal:8.0.0 and is removed from drupal:9.0.0.

Yes I have done. That is in patch #88

It's just more grammatically correct to say "Rely on Twig's auto-escaping feature, or use..." because then both parts start with an imperative verb.

Yes I agree. That is what I reverted to, that is what patch #88 has.

Meanwhile, removing the rest.

No sure what this means?

Good spot with the @link in the @trigger_error. There may be several of those and yes they need to be fixed in another issue.

So ... what 'needs work' is required on patch #88?

jonathan1055’s picture

Title: [backport] Fix remaining @deprecated (missing a change record) manually and enable the coding standard » [backport] Fix remaining @deprecated manually and enable the coding standard
Issue summary: View changes
Status: Needs work » Reviewed & tested by the community

Setting this to RTBC for patch #88 as I think the issue was set to 'Needs Work' unintentionally. In #89 it was marked RTBC and I have answered all the quesitons in #90. The patch still appiles and passes all green. It would be good to get this in and committed to 8.9 which will enable the basic @deprecated standard check.

We have a release note - see #54 so the addition of "missing change record" from #29 may not be required anymore.

If we delay much longer then the benefit will be lost. We've been so close since xjm's review in #56 but some noise, mis-understandings and out-of-scope change have obscurred the progress (I have been the culprit here, too).

xjm’s picture

Version: 8.9.x-dev » 9.0.x-dev
Status: Reviewed & tested by the community » Fixed

@jonathan1055 Yeah sorry about that; that comment was posted exactly to the minute when we started publishing Wednesday's security release. I think I clicked "save" without meaning to at the time, and then afterward did not have a chance to revisit the tab. I think what I was trying to say was that committing the rest was not possible at the time due to the code freeze, and that it was TBD whether we could still make the change in 8.9. Which we for sure can't now. :) So marking fixed against 9.0.x, but saving credits for the work on the backport too. Thanks!

xjm’s picture

Title: [backport] Fix remaining @deprecated manually and enable the coding standard » Fix remaining @deprecated manually and enable the coding standard
jonathan1055’s picture

it was TBD whether we could still make the change in 8.9. Which we for sure can't now.

Are you really sure this can't be committed to 8.9? It is not making any change to a single line of active code, only @deprecated doc commenting. The whole point of this is to get the standard sniff enabled on D8.9 to make it easier for code scans, etc. If this is not done it renders the entire standard menainglesss if not enforced.

xjm’s picture

@jonathan1055, 8.9 is now a production branch and is not allowed to have any changes to deprecations anymore. New deprecations will only go into 9.1.x, so that's where it's important to have the rule enabled -- and we do! Finally! So that is good news. :)

Both enabling a new phpcs rule and changing a deprecation message in any way are disruptions that aren't allowed from RC on, but on top of that, we aren't going to ever add new deprecations to 8.9. So even if we broke two separate rules of our allowed changes policy to commit this patch, we wouldn't really be getting any future benefit out of it, since there won't be any new deprecations we need to check.

I should have made it clear earlier that we were racing against the clock a bit, sorry. That's what the "rc deadline" tag that I added back in the end of April meant: This issue had a deadline of the code freeze of the first release candidate for each branch it was committed to. That originally would have been May 4 or so for both 9.0 and 8.9. We postponed it because we didn't want to release the first release candidate without SA-CORE-2020-003 and a few other things being resolved. 8.9.0-beta3 was released May 17, SA-CORE-2020-003 finally went out on May 20, and the 9.0 and 8.9 RCs were released on May 22, and so we were in code freeze most of that week for all those releases.

For more information, see:

Hope this helps! TLDR it's too late for 8.9, but we also didn't totally need it in 8.9 anyway. It was just a nice-to-have to fix up the inconsistent messages in our LTS branch, but it's OK also that we didn't get there before the branch went into RC.

jonathan1055’s picture

Thanks for the info and for explaining it. I did feel very deflated when I discovered it was not going to be committed in 8.9, having put so much effort and so many hours of my free time first writing the sniffs in Coder then working on the automated fixes, then these final few manual ones. (I am not sponsored, and not employed to do this, everything is my own time, like many here I know). I suppose you could say it was my fault not to have known what the 'rc deadline' tag date was. In your review on 19th May "mostly just some small formatting feedback" if you had mentioned the deadline I would have worked to meet it, but instead I encouraged the contributions of others, as is the Drupal way, and let people who wanted to help learn. There was lots of wasted time, but it did not seem to matter as people were gaining experience. Had I known the deadline I would have got the patch ready for re-review immediately. But ... that's all history now.

At least we have this in 9.0 and 9.1 so our efforts were not wasted. Right, now on to #3048495: Fix Drupal.Semantics.FunctionTriggerError coding standard

xjm’s picture

Yeah @jonathan1055 I totally understand -- this sniff has literally taken years for us to adopt. And it can be disappointing when extra work doesn't get used, like we had for those past couple weeks. That's part of why I made sure everyone who worked on the backport still received credit even though the backport wasn't actually committed at the end of the day. Thanks for your understanding and commitment to this chain of issues!

jonathan1055’s picture

Thank you :-)

Interesting that you said on 25th May in #93 above that no more changes could be made to core 8.9 but on #3138591: [D8 only] Add missing E_USER_DEPRECATED to deprecation notices there are commits to 8.9 yesterday, 28th May. The priority of that issue is only 'normal' and it is changing actual active code @trigger_error(... , E_USER_DEPRECATED).

I have read https://www.drupal.org/core/d8-allowed-changes but I'm confused now about what is and what is not allowed to be changed.

xjm’s picture

@jonathan1055 Yep, those were wrong changes that I also noticed just now when curating issues for the release notes (that issue and about 3 others also). We've actually just restored the rulesets from 9.0.0-rc1 and 8.9.0-rc1 because of this:
https://git.drupalcode.org/project/drupal/-/commit/6144e3b1f69b5ff9c786c...
https://git.drupalcode.org/project/drupal/-/commit/2a5cd8c8adcdae777324d...

Status: Fixed » Closed (fixed)

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

pameeela’s picture

Issue tags: +Bug Smash Initiative
jonathan1055’s picture

@pameeela Why add the tag to this issue which was fixed 6 weeks ago?

pameeela’s picture

Issue tags: -Bug Smash Initiative

Ah sorry, this was a mistake. We’re using the tag to track initiative stats so I updated tickets I worked on, but this was pre-bug smash so wasn’t one of them. Removed! Thanks for raising it.

quietone’s picture

Issue tags: -Needs followup

A followup was asked for in #90 concerning the use of @link/@endlink in the trigger error message. That message was removed from core in Drupal 9.0, which is no longer supported. Therefore, removing the tag.