Modifying the existing Features Override module to use the new Alter-hook method (A new paradigm for overriding Drupal features) would likely break any site using the existing version of Features Override. I don't see any good way to maintain backwards compatibility. So rather than patching the 7.x-1.x-dev version, I plan to create a new 2.x-dev branch to pursue the new alter method.

The plan is to determine a more general way to modify the core Features to place the needed alter hooks, then make the new 2.x Features Override work with these hooks to create new types of overrides.

Key goals for this project:

  1. Easier to create Overrides. Avoid the extra step of creating a new machine name, then creating a new feature to contain the overrides. Should be able to combine this into a single step to create an override feature.
  2. Enable/Disable Override should work as expected. If an override feature is disabled, the previous state of the base feature should be restored. When the override is enabled, the new overridden behavior should occur. #1316926: Disabling an Override does not restore the original settings
  3. Proper marking of "Overridden". When a change is made to settings that are present in both a base feature and an override feature, the override feature should be marked as Overridden, not the base feature. Reverting the Override feature will restore the previous state. #1314694: Components show as overridden after overrides are exported
  4. New UI. A new UI to select which overrides to include in a new Override Feature. The UI should also handle recreating the Override, adding items to the Override, removing items from the Override. #1314680: Provide a method to update a current override
  5. Handling multiple types of Overrides. forking vs subsets. #1316248: Enable overriding (forking) of whole component or a property/subset
  6. Minimal Features changes. Minimize changes needed to core Features module. For example, a single alter hook, rather than an alter hook in each exportable. #1314700: Use a universal hook for altering default components
  7. Intuitive code. If possible, a more intuitive exportable code for the Override could be generated.

An alternative is to create a new 2.x branch of Features itself that contains the Override functionality. For now I will pursue keeping Features Override as a separate module, with possible merging with Features in the future. Keeping the new development here where the community is focused on the Override functionality (instead of other Features code issues) would be useful.

Comments

nedjo’s picture

nedjo
he/him/his
Primary language English
commented

Thanks for taking the initiative here. I think a new 2.x branch introducing this new functionality is indeed the way to go. But I'm not convinced we should drop the existing approach.

While different in significant ways, the two approaches (the current one and the proposed one) have some things in common:

  • Both export override data.
  • Both use an alter hook or hooks to implement those overrides.

There are I think three distinct issues or questions here:

  1. How to structure and store override data? Here, we have two basic approaches: (a) determine and store the difference and (b) fork/override the full component or a defined subset. It seems like there are valid use cases for both.
  2. What point to do the override? The current solution intervenes at the default hook alter stage, whereas for the new approach a different intervention point is being considered. If that different point is better, it could be used as well for the current override data structure as for the new one. That is, since both data structures use an alter approach, solutions to #1316926: Disabling an Override does not restore the original settings and #1314694: Components show as overridden after overrides are exported should work equally well for the diff as for the fork data structure.
  3. UI and usability. Here there are steps that could be taken to improve the current implementation, e.g., #1314646: Provide default names and titles for overrides, #1314662: Provide drush and UI methods for mass exporting overrides, #1314680: Provide a method to update a current override.

The key question here is: is there a strong use case for the current (diff) approach that would not be sufficiently covered by the proposed new one. I outlined some of this in http://www.agileapproach.com/blog-entry/new-paradigm-overriding-drupal-f...

The current diff-type implementation is highly suitable to capturing small tweaks to an existing component. Because it adds and deletes but doesn't fork, it will retain any changes made to the original component through an upgrade. It's also universal--it doesn't require deciding in advance which point to enable overrides.

So, in the short term, I'm for:

What do you think Mike?

mpotter’s picture

mpotter commented

Yep, I think that mostly matches where I want to go with this.

Actually, with the addition of the drupal_alter call in the features_get_normal you might find that the existing Features Override module works better. Or that we can improve it to work better. I think that was the key to getting stuff like the Overridden working and the enable/disable.

So it turns out our two methods for doing Overrides are much closer than we thought. I didn't need to put the alter hooks in the exportables...we just both needed the additional alter hook in features_get_normal.

But one of the other things I'd still like to see in the 2.x branch is a different exportable code format for the Override. Right now the current object structure is very verbose and can be difficult to follow. I haven't played with your array diff code much yet to see how else I might do this, but it is something I want to work on. That would be the main reason to branch to 2.x if the exportable format is significantly different.

nedjo’s picture

nedjo
he/him/his
Primary language English
commented

Sounds good. You might want to have a look at the patches by cdale in #1309312: Maintain component ordering. (i.e. views fields) and #1085880: [PHP] warning: Attempt to assign property of non-object, which address current issues in overrides and also affect the structure of exports.

mpotter’s picture

mpotter commented

Nope, it turns out that just using the alter hook added to features_get_normal doesn't help with Features Override. The problem is with the fact that Features Override processes the alters itself rather than writing the alter code to the specific override feature themselves. So getting the 1.x version of Features Override to work even with the new alter hooks in Features will still be a significant architecture change I think.

mpotter’s picture

mpotter commented

Oh, and also I might be naive at this point, but it would be nice to come up with a way to do overrides without writing files to the public: area and without needing to store stuff in a database table. The Field Override is able to do this, so maybe it can be generalized. Still going through the Features Override code trying to understand it all. But I think it's related to the extra step of creating an override and then putting them into a feature as a second step.

Can't we just use the existing diff code from Features Override and export the differences to a feature like Field Override does? So instead of having field display settings arrays in the exported code we have individual statements that override parts of whatever array/object is being overridden.

nedjo’s picture

nedjo
he/him/his
Primary language English
commented
Title: 2.x Features Override using new Alter method? » 2.x Features Override using new Alter method

Yes, all worth exploring. If the easiest approach is ripping out all the existing code and starting fresh in a 2.x branch, please feel free. I'll be happy so long as we still meet the use case of selectively making the specific changes/additions and deletions to a component, as well as the one of overriding/forking it or a subset of it.

mpotter’s picture

mpotter commented

See the new sandbox: http://drupal.org/sandbox/mpotter/1366652 for the first attempt at a Features Override 2.x release. I haven't put it here yet because I'm still trying to decide if this should continue as a separate module or be moved into Features itself as core functionality.

scottrigby’s picture

scottrigby
he/him
Primary language English
Location Brooklyn, NY
commented
Status: Active » Fixed

@mpotter & @nedjo - this is fixed now, since it's literally merged in & the 7.x-2.x branch is the supported one on the project page (and 7.x-2.0-alpha1 is recommended tag)

scottrigby’s picture

scottrigby
he/him
Primary language English
Location Brooklyn, NY
commented
Version: 7.x-1.x-dev » 6.x-1.x-dev
Assigned: mpotter » scottrigby

I think we should make a 6.x-2.x backport, so we can keep progress on 7.x and 6.x in sync.

scottrigby’s picture

scottrigby
he/him
Primary language English
Location Brooklyn, NY
commented
Status: Fixed » Patch (to be ported)
scottrigby’s picture

scottrigby
he/him
Primary language English
Location Brooklyn, NY
commented
Status: Patch (to be ported) » Fixed

* 6.x-2.x 37a0613 New 6.x-2.x branch - backport from 7.x-2.x. FIrst pass, with only the most basic changes (core version, menu paths, theme function equivalent, remove image module code, js attach, README caveat)

Just chatted w hefox about a few Features issues that need backporting to 6.x for features_override-6.x-2.x to work properly:
#1437370: . is being mapped to ord(',') instead of ord('.') in features_dom_encode_options/features_dom_decode_options
#1317054: Provide support for exporting of altering of components
#984472: Add hook_node_info_alter

scottrigby’s picture

scottrigby
he/him
Primary language English
Location Brooklyn, NY
commented
Version: 6.x-1.x-dev » 6.x-2.x-dev

Rerolled patches for the three Features issues above (although the issue filter doesn't reflect the updated status yet - not sure what's up with that). And created a dev release for this new branch, so we can start tracking issues against it.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

(not verified) commented
Issue summary: View changes

update list formatting