@Rob_Feature I think where we currently have a module (uc_event_registration) and a feature (uc_event_registration_feature), we could have just one feature with a custom .module file. I think this is neater and would work: any thoughts? (I'm not a Features expert btw, so I may have missed something obvious!)

Comments

Rob_Feature’s picture

Rob_Feature CreditAttribution: Rob_Feature commented

Currently we have a module which requires the feature to be enabled. This is so the feature will then enable all the modules IT requires. If we had it so the feature required the module, that would be fine, but it would be a little more confusing to install because, well, people have to 'get' what features are and how to enable them.

When building it I figured people know how to enable normal modules, so having a 'normal' module in the Ubercart section of the module list be the thing that makes everything else happen seemed like the most logical and clearest approach.

Also, Features dont normally include module files...they, instead, require other modules to be enabled. When outputting a feature from the UI feature-builder, we wouldn't be able to output our custom .module that way, so you could never update the feature, really. Currently we can update the feature and drop it into the existing module, giving a little flexibility to edit the feature.

Let me know if that makes sense. (it's all basically just philosophy of features stuff, as I understand it)

AndyF’s picture

AndyF CreditAttribution: AndyF commented

Hi Rob_Feature,

Sorry for the delay, spent a fantastic week on holiday in St. Petersburg Russia - georgious! Now back to work :p

Currently we have a module which requires the feature to be enabled ... If we had it so the feature required the module, that would be fine, but...

I'm suggesting having only a feature - no kosher module whatsoever.

Also, Features dont normally include module files

But they do allow for custom .module files - from the Features project page:

You can add custom code (e.g. custom hook implementations, other functionality, etc.) to your feature in myfeature.module as you would with any other module. Do not change or add to any of the features .inc files unless you know what you are doing. These files are written to by features on updates so any custom changes may be overwritten.

When you recreate the feature, your custom code will come with it. So I think the idea of having a feature with custom code is compatible with the Features philosophy. Btw what started this all off for me, was at a certain point while playing around I found myself in a state with uc_event_registration_feature enabled but uc_event_registration not. This will happen every time someone enables then disables the module. For me, these should not be separate because they're both part of the same explicit chunk of functionality that we want to add (ie a specific Ubercart webform class).

Does that make sense?

Rob_Feature’s picture

Rob_Feature CreditAttribution: Rob_Feature commented

Yeah, this makes sense! I didn't know you could add custom .module files. I'd say go ahead and mock it up and if everything works smoothly (and there's no upgrade path issues) go for it. I agree that combining them into a single thing is ideal!

AndyF’s picture

AndyF CreditAttribution: AndyF commented
Assigned: Unassigned » AndyF

Willco, it should be relatively simple. The upgrade scenario does worry me a bit. We can't just uninstall the 'old' feature in hook_update_N, as by that time the old feature should have been deleted as the first part of the upgrade. The only way I can think of would be to make a 2.x branch and use hook_requirements to ensure that the old feature's not enabled before using the new one. Not pretty, but I can't think of another safe way. Any thoughts/ideas?

AndyF’s picture

AndyF CreditAttribution: AndyF commented
Status: Active » Closed (won't fix)

I can't think of any way of handling the upgrade that wouldn't result in an empty 'stub' module hanging around afterwards, which I think's uglier than having two modules, so I'm gonna leave things as is.