Problem/Motivation

Part of #3524379: [meta] Remove usage of history module from comment module

Steps to reproduce

Proposed resolution

  • Remove history integration from \Drupal\comment\Hook\CommentViewsHooks::viewsDataAlter and move to \Drupal\history\Hook\HistoryViewsHooks::viewsDataAlter
  • Move \Drupal\Tests\comment\Functional\Views\NodeCommentsTest::testNewComments to \Drupal\Tests\history\Functional\Views\CommentsHistoryTest::testNewComments
  • Move test_new_comments view to new history_test_views module

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3542847

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:

About issue forks

Comments

mstrelan created an issue. See original summary.

mstrelan opened merge request !13071

mstrelan’s picture

mstrelan commented
Status: Active » Needs review
smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

If history is removed in 12 shouldn't the deprecations be for 12? Else in 12 we would have core code calling a contrib module.

If you are another contributor eager to jump in, please allow the previous poster(s) at least 48 hours to respond to feedback first, so they have the opportunity to finish what they started!

mstrelan’s picture

mstrelan commented

Yeah that makes sense, updated.

FWIW this can move to history module regardless of if we complete the whole deprecation in time or not.

mstrelan’s picture

mstrelan commented
Status: Needs work » Needs review
mstrelan’s picture

mstrelan commented
Issue summary: View changes
smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Fair, like to think we can get it out in time! Rest of the changes look good. Any cleanup in history can be by that maintainer in contrib.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

This looks good. Not entirely sure we need the CR for the plugin deprecation because the plugin ID hasn't changed, but went ahead and added a small note to that and published it. Committed/pushed to 11.x, thanks!

  • catch committed e74abad5 on 11.x 
    Issue #3542847 by mstrelan, smustgrave: Move comment/history/views...

catch closed merge request !13071

catch’s picture

catch
he/him
Primary language English
commented

Status: Fixed » Closed (fixed)

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