Problem/Motivation

Part of #3518993: [META] Replace comment constants with enums

Steps to reproduce

Proposed resolution

CommentItemInterface - FORM_SEPARATE_PAGE and FORM_BELOW moves to a FormLocation enum with cases SeparatePage, and Below.

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#4 3550054-nr-bot__s6kxzll.txt91 bytesneeds-review-queue-bot

Issue fork drupal-3550054

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 !13395

mstrelan’s picture

mstrelan commented
Status: Active » Needs review
needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3550054-nr-bot__s6kxzll.txt91 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

mstrelan’s picture

mstrelan commented
Status: Needs work » Needs review

Rebased since #3547353: Move HIDDEN, CLOSED and OPEN constants in CommentItemInterface to new CommentingStatus enum was committed

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Small Phpcs problem mind taking a quick look @mstrelan

mstrelan’s picture

mstrelan commented
Status: Needs work » Needs review

Umami performance test failing not sure if it's related?

mstrelan’s picture

mstrelan commented

Test fail is happening elsewhere, opened #3578666: Fix AssetAggregationAcrossPagesTest

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
Issue tags: +Needs Review Queue Initiative, +Needs change record updates

Applied the MR

Did a search for CommentItemInterface::FORM_BELOW = 0 hits
Did a search for CommentItemInterface::FORM_SEPARATE_PAGE = 0 hits

Seems like a good conversion but the CR is empty. Will keep my eye out for this one more quickly.

mstrelan’s picture

mstrelan commented
Status: Needs work » Needs review
Issue tags: -Needs change record updates

Updated CR, thanks

smustgrave’s picture

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

Thanks! Rest looks good.

godotislate made their first commit to this issue’s fork.

godotislate closed merge request !13395

  • godotislate committed 90bc2112 on main 
    refactor: #3550054 Move FORM_SEPARATE_PAGE and FORM_BELOW constants in...

  • godotislate committed 9a2fc401 on 11.x 
    refactor: #3550054 Move FORM_SEPARATE_PAGE and FORM_BELOW constants in...
godotislate’s picture

godotislate
he/him
commented
Version: main » 11.x-dev
Status: Reviewed & tested by the community » Fixed

I bumped the deprecation version to 11.4 and the removal version to 13. Since these were minor comment changes, I applied the suggestions myself and committed 90bc211 and pushed to main.

There was a merge conflict cherry-picking to 11.x in core/modules/comment/src/CommentLinkBuilder.php. This only had to do with use statements, so I manually resolved and committed 9a2fc40 and pushed to 11.x. Thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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