Updated: Comment #0

Problem/Motivation

There's a some type-hinting errors and typos

Proposed resolution

Use proper type-hinting

User interface changes

no

API changes

no

CommentFileSizeAuthor
#4 interdiff.txt1.75 KBandypost
#4 drupal8.comment-module.2078139-4.patch5.06 KBandypost
#2 interdiff.txt846 bytesandypost
#2 drupal8.comment-module.2078139-2.patch4.3 KBandypost
#1 drupal8.comment-module.2078139-1.patch4.08 KBandypost

Comments

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Active » Needs review
StatusFileSize
new drupal8.comment-module.2078139-1.patch4.08 KB

At least that's enough

andypost’s picture

andypost
he/him
Primary language Russian
commented
StatusFileSize
new drupal8.comment-module.2078139-2.patch4.3 KB
new interdiff.txt846 bytes

And another few places

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented 
+++ b/core/modules/comment/lib/Drupal/comment/Tests/CommentTestBase.php
@@ -81,8 +80,8 @@ function setUp() {
+   * @param \Drupal\Core\Entity\EntityInterface|null $node

If we made this $entity here, and removed references to nodes, would that make comment-field tests for other than node easier?

Other than that, looks good to me.

andypost’s picture

andypost
he/him
Primary language Russian
commented
StatusFileSize
new drupal8.comment-module.2078139-4.patch5.06 KB
new interdiff.txt1.75 KB

Done

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented 
+++ b/core/modules/comment/lib/Drupal/comment/Tests/CommentTestBase.php
@@ -80,7 +80,7 @@ function setUp() {
    *   Node to post comment on or NULL to post to the previously loaded page.

Still refs node in comments?

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Needs review » Reviewed & tested by the community

On second thoughts, referencing other than nodes is premature

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Looks good. There are some API changes here, but they only bring this class more in-line with what we do elsewhere, and shouldn't break any existing code.

Committed and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

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