Remove the hook_uninstall() implementations that are not necessary

[avpaderno]

hook_uninstall() implementations that remove a table defined in hook_schema() or that delete the configuration values associated with the module should be removed. Backdrop already does that in backdrop_uninstall_modules().

[argiepiano]

Can you provide specific examples of this hook in the Examples project? At times hook_uninstall() may be needed for other things.

[avpaderno]

I am referring to the following hook_uninstall() implementations.

/**
 * Implements hook_uninstall().
 *
 * This removes the example data when the module is uninstalled.
 *
 * @ingroup tabledrag_example
 */
function tabledrag_example_uninstall() {
  db_drop_table('tabledrag_example');
}

The module implements hook_schema() which defines that table's schema.

/**
 * Implements hook_uninstall().
 *
 * It's good to clean up after ourselves
 *
 * @ingroup tablesort_example
 */
function tablesort_example_uninstall() {
  db_drop_table('tablesort_example');
}

The same is true for this module.

/**
 * Implements hook_uninstall().
 *
 * @ingroup block_example
 */
function block_example_uninstall() {
    $config = config('block_example.settings');
    $config->set('block_example_string', 'Some example content.');
    $config->save();
}

The same function in the Drupal 7 example modules deletes a persistent variable, but the equivalent code in Backdrop is not necessary.
This hook is setting a configuration value right before the module is uninstalled. I thought there were another hook that would check the value set in this hook, but the block_example.install file does not contain any other hook.

[argiepiano]

I agree that the first two examples should be removed.

The third example looks like a typo to me. Wondering if the person who ported this intended to implement hook_install() but instead typed hook_uninstall()? The code inside looks like it's initializing the content of that config file. If this is supposed to go into hook_install() instead, then we also need to implement hook_config_info() in the block_example.module, so that the config file gets removed. Right now the behavior is completely strange: upon uninstalling the module, it creates a config file that will remain in the config directory forever.

[avpaderno]

Configuration values are not used by the Block Example module. It does not even need them, since all the block settings come from a hook_block_configure() implementation or a class that extends Block.

[avpaderno]

I would say that the full block_example.install file can be safely removed. The presence of that code in block_example_uninstall() is quite a bug.

[argiepiano]

Yes