Problem/Motivation

In #2887142: NodeType source plugin should include comment information We deprecated comment variable sources in favor of integrating them into NodeType. In order to preserve BC, so code was added to the Comment source, for migrations that might still be using the deprecated sources. This code was encapsulated in a method that was incorrectly marked as deprecated. Deprecated code should not be used anywhere in core. This code is a necessary BC shim intended to be used in Drupal 8 but unneeded in Drupal 9. Therefore this code should not be deprecated until it is no longer needed, i.e. in Drupal 9, and then removed in Drupal 10.

Proposed resolution

Undeprecate the method and file a follow-up issue to deprecate the code in D9, and comment the code with @todos referencing the followup.

Remaining tasks

Do it

User interface changes

none.

API changes

Previously deprecated method Comment::prepareComment() is now marked @internal

Data model changes

none

Release notes snippet

none

CommentFileSizeAuthor
#11 interdiff.3069055.8-11.txt717 bytesmikelutz
#11 3069055-11.drupal.Properly-deprecate-DrupalcommentPluginmigratesourced6CommentprepareComment.patch1.55 KBmikelutz
#8 interdiff.3069055.6-8.txt1.46 KBmikelutz
#8 3069055-8.drupal.Properly-deprecate-DrupalcommentPluginmigratesourced6CommentprepareComment.patch1.41 KBmikelutz
#6 3069055-6.drupal.Properly-deprecate-DrupalcommentPluginmigratesourced6CommentprepareComment.patch1.38 KBdhirendra.mishra
#6 interdiff_3_6.txt580 bytesdhirendra.mishra
#3 3069055-3.drupal.Properly-deprecate-DrupalcommentPluginmigratesourced6CommentprepareComment.patch1.38 KBmikelutz
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mikelutz created an issue. See original summary.

mikelutz’s picture

mikelutz
He/Him
Primary language English
Location Michigan, USA
CreditAttribution: mikelutz as a volunteer and at ICON Interactive commented

This method should actually be marked as internal.

mikelutz’s picture

mikelutz
He/Him
Primary language English
Location Michigan, USA
CreditAttribution: mikelutz as a volunteer and at ICON Interactive commented
Title: Properly deprecate Drupal\comment\Plugin\migrate\source\d6\Comment::prepareComment » Undeprecate Drupal\comment\Plugin\migrate\source\d6\Comment::prepareComment() and mark as internal.
Issue summary: View changes
Status: Active » Needs review
FileSize
3069055-3.drupal.Properly-deprecate-DrupalcommentPluginmigratesourced6CommentprepareComment.patch1.38 KB

This patch should only change documentation, not functionality.

quietone’s picture

quietone CreditAttribution: quietone commented
Status: Needs review » Needs work

Just a typo

+++ b/core/modules/comment/src/Plugin/migrate/source/d6/Comment.php
@@ -45,7 +49,12 @@ public function prepareRow(Row $row) {
+   * @todo Remove this method and its use in in drupal:9.0.0.

s/in in/in

dhirendra.mishra’s picture

dhirendra.mishra CreditAttribution: dhirendra.mishra at Valuebound for Valuebound commented
Assigned: Unassigned » dhirendra.mishra

Let me do it

dhirendra.mishra’s picture

dhirendra.mishra CreditAttribution: dhirendra.mishra at Valuebound for Valuebound commented
Assigned: dhirendra.mishra » Unassigned
Status: Needs work » Needs review
FileSize
interdiff_3_6.txt580 bytes
3069055-6.drupal.Properly-deprecate-DrupalcommentPluginmigratesourced6CommentprepareComment.patch1.38 KB

Pls review updated patch

mikelutz’s picture

mikelutz
He/Him
Primary language English
Location Michigan, USA
CreditAttribution: mikelutz as a volunteer and at ICON Interactive commented
Status: Needs review » Needs work

I think this needs to be handled a bit differently. patch coming soon.

mikelutz’s picture

mikelutz
He/Him
Primary language English
Location Michigan, USA
CreditAttribution: mikelutz as a volunteer and at ICON Interactive commented
Title: Undeprecate Drupal\comment\Plugin\migrate\source\d6\Comment::prepareComment() and mark as internal. » Undeprecate Drupal\comment\Plugin\migrate\source\d6\Comment::prepareComment()
Issue summary: View changes
Status: Needs work » Needs review
FileSize
3069055-8.drupal.Properly-deprecate-DrupalcommentPluginmigratesourced6CommentprepareComment.patch1.41 KB
interdiff.3069055.6-8.txt1.46 KB

After further thought and discussions, this code is used in Drupal 8, and the @internal tag is not realistically respected, so we don't want to just remove an internal method. The actual correct solution here is to simply remove the @deprecated tag, as the method is in use and is therefore not yet deprecated.

When the Drupal 9 branch is opened, the calls to this method can be removed, and it can be deprecated at that time for removal in Drupal 10.

heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn at MTech, LLC commented
Status: Needs review » Reviewed & tested by the community

Removing a deprecation seems fine here.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented
Status: Reviewed & tested by the community » Needs review 
+++ b/core/modules/comment/src/Plugin/migrate/source/d6/Comment.php
@@ -34,18 +34,29 @@ public function query() {
+   * @throws \Exception

we need to document when the exception can occur, thanks!

mikelutz’s picture

mikelutz
He/Him
Primary language English
Location Michigan, USA
CreditAttribution: mikelutz as a volunteer and at ICON Interactive commented
FileSize
3069055-11.drupal.Properly-deprecate-DrupalcommentPluginmigratesourced6CommentprepareComment.patch1.55 KB
interdiff.3069055.8-11.txt717 bytes
heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn at MTech, LLC commented
Status: Needs review » Reviewed & tested by the community

And back to RTBC.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Reviewed & tested by the community » Fixed

Committed 239ce7f and pushed to 8.8.x. Thanks!

  • alexpott committed 239ce7f on 8.8.x 
    Issue #3069055 by mikelutz, dhirendra.mishra, quietone, larowlan:...

Status: Fixed » Closed (fixed)

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