features_revert() missing docblock (patch) [#1651330]

It seems that features_revert() is intended as a convenience function, but its parameters and behavior are not self-explanatory.

I saw a bunch of issues about how this function works (for example, #929954: Call features_revert() with no params reverts only modules with hook_features_rebuild() function), but the first step is probably to document what it does right now. Here's my suggestion:

/**
 * Wrapper around _features_restore().
 *
 * @param array $revert
 *   An array describing the features to act on. The keys of this array are
 *   feature machine names, and the values are arrays of component types. For
 *   example, passing the following array would revert parts of features named
 *   'custom_node_page' and 'custom_page_list_view':
 *
 *   @code
 *     $revert = array(
 *       'custom_page_node' => array(
 *         'field',
 *         'variable',
 *       ),
 *       'custom_page_list_view' => array(
 *         'views_view',
 *       ),
 *     );
 *  @endcode
 *
 *  You must specify all of the components that should be reverted; if no
 *  components are present, no part of the feature will be reverted.
 */
function features_revert($revert = array()) {
  return _features_restore('revert', $revert);
}

A patch is attached.

Comment	File	Size	Author
	features-features_revert_docblock.patch	1.03 KB	becw

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

hefox CreditAttribution: hefox commented 24 June 2012 at 05:28

Status:

Needs review

» Needs work

Thanks + good idea, but how about including what happens when called with no arguments?

"passing the following array would revert parts of features named 'custom_node_page' and 'custom_page_list_view'"

This part is confusingly worded; e.g. unclear whether 'custom_node_page' is part of a feature or feature itself (it's the later).

Comment #2

becw CreditAttribution: becw commented 26 June 2012 at 16:39

Honestly, I'm not sure what happens when it's called with no arguments! From reading the code, it seems like nothing happens when the function is called with no arguments, hence the sentence at the end:

/**
 * ...
 *  You must specify all of the components that should be reverted; if no
 *  components are present, no part of the feature will be reverted.
 */

I agree with you about the confusingly worded bit--I'll try and figure out another way to say that.

Comment #3

hefox CreditAttribution: hefox commented 26 June 2012 at 17:02

Without checking, I believe it finds anything in rebuild status and rebuilds it (same thing cron calls, I believe). Rebuild status is when "Dude, that code has totally changed, but not the database", vs. override "Dude, the code is totally the same, but the damn database has totally different, man".