When running aka testing #375397: Make Node module optional

drush site-install --yes
drush pm-disable node comment --yes
drush pm-uninstall comment --yes

the last command fails with
PHP Fatal error: Call to undefined function node_type_get_types() in /Users/clemens/Sites/drupal/d8/www/core/modules/comment/comment.install on line 17

CommentFileSizeAuthor
#3 comment-1426062-3.patch1.31 KBtim.plunkett
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

naxoc’s picture

In hook_uninstall() the module cleans up variables like this:

  $node_types = array_keys(node_type_get_types());
  foreach ($node_types as $node_type) {
    field_attach_delete_bundle('comment', 'comment_node_' . $node_type);
    variable_del('comment_' . $node_type);
    variable_del('comment_anonymous_' . $node_type);
    variable_del('comment_controls_' . $node_type);
    variable_del('comment_default_mode_' . $node_type);
    variable_del('comment_default_order_' . $node_type);
    variable_del('comment_default_per_page_' . $node_type);
    variable_del('comment_form_location_' . $node_type);
    variable_del('comment_preview_' . $node_type);
    variable_del('comment_subject_field_' . $node_type);
  }

A lot of comment modules variables are postfixed with node types, so what do we do? Let node module clean them up? Leave them? Delete based on the first part of the variable name?

naxoc’s picture

Come to think of it - if the variables are going in CMI-config files at some point, then this issue more or less solves it self.

tim.plunkett’s picture

Status: Active » Needs review
FileSize
1.31 KB

Here's a test.

Status: Needs review » Needs work

The last submitted patch, comment-1426062-3.patch, failed testing.

tim.plunkett’s picture

As of the latest patch in #731724: Convert comment settings into a field to make them work with CMI and non-node entities, this would be obsolete. Still worth fixing in case that doesn't land.

mtift’s picture

Issue tags: +blocked

10 months later this is still a problem and it is still blocked on #731724: Convert comment settings into a field to make them work with CMI and non-node entities

larowlan’s picture

Would love your help over there (see to-do list in issue summary).
We've been re-rolling it since November.

swentel’s picture

Status: Needs work » Closed (won't fix)

Comment is now a field