Attached patch fixes that.

CommentFileSizeAuthor
#14 939254_features_pipe_alter_module_name.patch1.35 KBhefox
#11 939254_features_pipe_alter_module_name.patch1.35 KBhefox
#6 features_pipe_omponent_alter-939254-5.patch768 byteshefox
#5 features_pipe_omponent_alter-939254-4.patch671 byteshefox
#4 features_pipe_omponent_alter-939254-3.patch1.62 KBhefox
features_component_alter.patch773 bytesfago
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

apaderno’s picture

apaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
CreditAttribution: apaderno commented
Status: Needs review » Reviewed & tested by the community

The patch does what it should do.

irakli’s picture

irakli CreditAttribution: irakli commented
Status: Reviewed & tested by the community » Postponed (maintainer needs more info)

Sorry for my ignorance, but can you please explain?

Features documentation (as described in features.api.php) declares the hook as: 

hook_features_pipe_component_alter(&$pipe, $data, $export)

so, there is no $module_name argument there.

Also signature of drupal_alter() is: 

drupal_alter($type, &$data, &$context1 = NULL, &$context2 = NULL)

while the patch suggests a fifth argument: $module_name.

Why?

Thank you.

fago’s picture

fago
Primary language German
Location Vienna
CreditAttribution: fago commented
Status: Postponed (maintainer needs more info) » Closed (works as designed)

Strange, I cannot find it in the docs any more. Anyway, as said drupal_alter() won't support it either.

hefox’s picture

hefox CreditAttribution: hefox commented
Title: hook_features_pipe_component_alter() doesn't match the docs » Remove of $module_name in hook_features_pipe_component_alter() breaks sites using hook, no upgrade path
Status: Closed (works as designed) » Needs review
FileSize
features_pipe_omponent_alter-939254-3.patch1.62 KB

Re-opening.

$module name existed in the d6 branch but was removed (without documentation that it was removed for d6) and there's no way to get that information, breaking any site using it (and I am using it).

It was removed via http://drupalcode.org/project/features.git/commit/af211d123d7048d8daee4a... because drupal_alter only supports 3 arguments.

then it was backported via cb72c5121251a950ad82008267e65740faface83, but drupal_alter supports any number of arguments so there was no reason to backport it other than to keep code consistant, as far as I can tell. (the commit message references wrong ticket number so can't find the issue that backported it).

So... can we just put back $module_name for d6 and add module_name to third argument (whichever way is preferable) to d7? Cause yea, when doing custom ****, that is really useful information.

hefox’s picture

hefox CreditAttribution: hefox commented
FileSize
features_pipe_omponent_alter-939254-4.patch671 bytes

Patch to features root, oops.

hefox’s picture

hefox CreditAttribution: hefox commented
FileSize
features_pipe_omponent_alter-939254-5.patch768 bytes

Actually this may be a better idea.

bleen’s picture

bleen CreditAttribution: bleen commented

subscribe

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented

Note that this will cross paths with #1300780: Provide an actual hook_features_pipe_alter() for general altering.

bleen’s picture

bleen CreditAttribution: bleen commented
Issue tags: +needs backport to 6.x

Note: Hefox is relying on this issue being fixed and backported to 6.x in order to resolve #792472: Implement node type -> strongarm pipe for Features

hefox’s picture

hefox CreditAttribution: hefox commented
Title: Remove of $module_name in hook_features_pipe_component_alter() breaks sites using hook, no upgrade path » Return the $module_name to hook_features_pipe_component_alter()
Category: bug » feature

Unfortunately, strongarm had the 'fix' committed already, without knowledge/reference of this or the other issue.

hefox’s picture

hefox CreditAttribution: hefox commented
FileSize
939254_features_pipe_alter_module_name.patch1.35 KB

Updating 7.x patch

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Status: Needs review » Reviewed & tested by the community

Looks okay to me. I guess the D6 version has to wait on #1300780-7: Provide an actual hook_features_pipe_alter() for general altering. again.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Status: Reviewed & tested by the community » Needs work

Ooops, should have double checked.

+++ b/features.export.incundefined
@@ -51,10 +51,12 @@ function _features_populate($pipe, &$export, $module_name = '') {
+      $export['module_name'] = $component;

$component should be $module_name

hefox’s picture

hefox CreditAttribution: hefox commented
Status: Needs work » Reviewed & tested by the community
FileSize
939254_features_pipe_alter_module_name.patch1.35 KB

(Copy and paste oops sneaked into the reroll)

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented

This is actually RTBC now.

mpotter’s picture

mpotter CreditAttribution: mpotter commented
Version: 7.x-1.x-dev » 6.x-1.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Commited a1a2ab4.

  • mpotter committed a1a2ab4 on 8.x-3.x 
    Issue #939254 by hefox, fago: Added Return the () to...