Problem/Motivation

In core/lib/Drupal/Component/Plugin/Attribute/Plugin.php:

   * @param string $id
   *   The attribute class ID.

This should say the plugin ID.

Proposed resolution

Fix docblock

Remaining tasks

Update docblock
Create MR

CommentFileSizeAuthor
#13 mr_fail.png180.01 KBlostcarpark

Issue fork drupal-3476226

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:

Comments

joachim created an issue. See original summary.

chrisdarke’s picture

Issue summary: View changes
Issue tags: +Barcelona2024

The Drupal Contribution Mentoring team is triaging issues for DrupalCon Barcelona 2024, and we are reserving this issue for Mentored Contribution during the event.

After September 27, 2024, this issue returns to being open to all. Thanks!

ekorzen2’s picture

I'm working on this at DrupalCon Barcelona 2024.

lostcarpark’s picture

I'm mentoring Emily at Barcelona.

lostcarpark’s picture

Randy Fay (rfay) has also helped.

ekorzen2’s picture

Status: Active » Needs review

Merge request created, needs review.

dc-mkinoshita’s picture

I'm working on this issue #barcelona2024

dc-mkinoshita’s picture

The text change looks correct to me.
It still needs technical review.

joachim’s picture

Status: Needs review » Needs work

> + * The attribute plugin ID.

This should not say 'attribute'.

The attribute is a class that is used to define the plugin's metadata. The values passed to the attribute constructor's parameters will be about the plugin, not the attribute.

ekorzen2’s picture

Status: Needs work » Needs review

Removed "attribute."

joachim’s picture

Status: Needs review » Reviewed & tested by the community

Perfect! Thanks!

lostcarpark’s picture

Status: Reviewed & tested by the community » Active
StatusFileSize
new180.01 KB

Thanks for working on this issue!

One thing we need to check before we commit this is that the merge request passes the test suite. You can see in the issue summary that the test suite has failed with the little red x in a circle:

Screenshot of MR information in issue

I clicked on the red X to review the tests, to take me to this page, and navigated to the Nightwatch test that failed: https://git.drupalcode.org/issue/drupal-3476226/-/jobs/2873651

Here is the section that failed:

  ️TEST FAILURE (4m 26s):  
   - 1 assertions failed; 1425 passed
   ✖ 1) Olivero/oliveroMobileMenuTest
   – Verify mobile menu works when Big Pipe when authenticated (14.162s)
   → ✖ NightwatchAssertError
   Timed out while waiting for element <form.system-modules [name="modules[big_pipe][enable]"]:disabled> to be present for 10000 milliseconds. - expected "found" but got: "not found" (10176ms)
    Error location:
    /builds/issue/drupal-3476226/core/tests/Drupal/Nightwatch/Commands/drupalInstallModule.js:39
    –––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––
     37 |     // Wait for the checkbox for the module to be disabled as a sign that the
     38 |     // module has been enabled.
     39 |     this.waitForElementPresent( 
     40 |       `form.system-modules [name="modules[${module}][enable]"]:disabled`,
     41 |       10000,
    –––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––
 Wrote HTML report file to: /builds/issue/drupal-3476226/nightwatch_output/nightwatch-html-report/index.html

As this is a JavaScript test, and the change is to documentation, it is very unlikely the change caused the failure.

I am rerunning the failed test to verify.

lostcarpark’s picture

There was also a functional test failure:

This is a link to the failed test run log: https://git.drupalcode.org/issue/drupal-3476226/-/jobs/2873642

There was 1 failure:
1) Drupal\Tests\block\Functional\BlockCacheTest::testCachePermissions
Behat\Mink\Exception\ResponseTextException: The text "k1dabd5n" was not
found anywhere in the text of the current page.
/builds/issue/drupal-3476226/vendor/behat/mink/src/WebAssert.php:907
/builds/issue/drupal-3476226/vendor/behat/mink/src/WebAssert.php:293
/builds/issue/drupal-3476226/core/tests/Drupal/Tests/WebAssert.php:979
/builds/issue/drupal-3476226/core/modules/block/tests/src/Functional/BlockCacheTest.php:157
FAILURES!
Tests: 5, Assertions: 80, Failures: 1.

I searched the Drupal Core issue log, and it appears related to this issue:

https://www.drupal.org/project/drupal/issues/3468830

As this is clearly not caused by this change, I am also rerunning this test.

joachim’s picture

Status: Active » Reviewed & tested by the community

I think that as this MR is a docs change only, we can be certain that the test failures are unconnected.

lostcarpark’s picture

Status: Reviewed & tested by the community » Active

Tests passed on rerun.

lostcarpark’s picture

Status: Active » Reviewed & tested by the community

Oops, cross posted, seems to have accidentally changed status. Setting back to RTBC.

rfay’s picture

Congratulations on getting this to RTBC!

quietone’s picture

Version: 11.x-dev » 10.4.x-dev
Status: Reviewed & tested by the community » Fixed

Committed to 11.x and to 10.4.x. Thanks!

  • quietone committed d2ca6921 on 10.4.x
    Issue #3476226 by ekorzen2, lostcarpark, joachim, dc-mkinoshita,...

  • quietone committed 49a3c28e on 11.x
    Issue #3476226 by ekorzen2, lostcarpark, joachim, dc-mkinoshita,...

quietone’s picture

Status: Fixed » Closed (fixed)

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