Problem/Motivation
Part of #2851705: [meta] Deprecate DRUPAL_DISABLED, DRUPAL_OPTIONAL and DRUPAL_REQUIRED - we want to deprecate DRUPAL_DISABLED, DRUPAL_OPTIONAL and DRUPAL_REQUIRED. Doing this in a single issue is too hard to review and we want to change the API as we go.
This ticket is for the Comment module.
Proposed resolution
- Add CommentPreview enum
- Replace usages of constants with new enum
- Update $mode param in CommentTestBase::setCommentPreview to take CommentPreviewMode enum, throw a deprecation when its an integer.
- $message is unused by setCommentSettings so we can remove $mode_text entirely. We can used named params to continue to pass $field_name. See #3478418: Deprecate $message param from \Drupal\Tests\comment\Functional\CommentTestBase::setCommentSettings
- Pass $mode->value into setCommentSettings
API changes
CommentTestBase::setCommentPreview now takes a CommentPreviewMode enum instead of int for $mode
Release notes snippet
N/A
Issue fork drupal-3538660
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:
- 3538660-add-commentpreview-enum
changes, plain diff MR !12866
Comments
Comment #2
acbramley commentedComment #4
acbramley commentedComment #5
mstrelan commentedLooks great, only 3 minor comments
Comment #6
acbramley commentedComment #7
mstrelan commentedHappy with that, will leave to a committer to decide on BC implications.
Comment #8
dwwTentatively agree this is RTBC, although I couldn't resolve the 2 open threads in good faith, and I even opened a 3rd one.
Everything else in the MR looks good to me. Very straight-forward conversion. Made a few tiny edits to the CR for formatting, but that also looks good and is clear.
Comment #9
dwwSaving credits.
Comment #10
catchThis looks good - I responded to and closed the open threads on the MR because in each case they didn't need any changes for me.
Committed/pushed to 11.x, thanks!
Comment #12
catchComment #14
catch