Removing non-required modules from .info / .make files

I agree :) Could you make a patch proposing which modules you want to leave out?

I personally don't have an opinion on this either way. But I'd prefer to hear from @populist before making this change, since he's the original author and the one who added all those modules in the first place.

co-signed. i feel panopoly should support one module for the admin and my vote is NAVBAR. as much as i love admin and admin tools, navbar is responsive and that seems to be the way this project is going, what with RADIX being adopted by major dev houses using Panopoly.

I'm definitely not opposed to this! However, I just want to point out that removing things from the .make files is tricky as it relates to upgrades. We can remove dependencies from .info with no ill effects, because that just changes what enabled after a fresh install.

But if, for example, we remove 'admin' in Panopoly 1.2 but someone has it enabled, once they upgrade it will mysteriously stop working. :-/

I'm not sure what the right solution is -- maybe we can implement a clever update function that will detect this and instruct the user to download the module to sites/all/modules before continuing with the upgrade? I haven't really had a chance to think about it yet.

I would say that removing things from the make files is ok as long as this is noted in bold on the release notes and that appropriate warnings are worked into the upgrade upgrade process somewhere.

I'm not sure that it would be possible to automatically move the module to sites/all/modules but if it is then that could be an option. I think telling people that it is no longer included as part of panopoly and that they should download and add to their sites module directory if they want to continue to use them is enough.

In the case of admin_menu, we are talking about replacing with navbar and so I would just consider that as an upgrade. In the case of defaultcontent, anyone using it is likely a distribution maintainer of sorts and so checking things like this should be a pretty familiar process for them as part of upgrading their distribution's panopoly base. I hadn't even realised respond.js was provided, and I'm not sure about uuid... As a base distribution I feel like we should be providing something as fundamental as UUID support so I'm not convinced that we should be removing this one. I'd prefer to try and support it properly.

Does the module get removed from the profiles/panopoly/modules/contrib folder during an upgrade if it's not in the .make file anymore?

Yes - because the best practice instructions for upgrading a distribution involve downloading the newer package and replacing all directories except the /sites in your codebase.

See http://www.drupalcommerce.org/commerce-kickstart-2/install for some good notes on upgrading a distribution (we should think about adding something like this to our documentation too if it doesn't already exist)

I'd agree that admin/admin_menu -> navbar is an upgrade, but if we remove the admin/admin_menu modules, there's no way to uninstall them and hence they could leave stale data around (pretty much just left over variables in the case of admin_menu).

As to uuid: there was a fix in Ctools (huzzah!) to resolve the conflict. :-) But, I still think it's worth discussing it's removal from panopoly.info. I'm not sure what it's use case is in the context of Panopoly. We're not going to use features_uuid -- we'd use migrate for that! Services/Deploy/etc are outside the scope of Panopoly itself and more for a distribution built on it... Anyway, we can keep talking about that in it's own issue!

re admin_menu - > navbar - thats true about the uninstalling part. Another option could be to leave admin_menu in the makefile but update the installer to only install navbar for new installs and add an advisory to the release notes of the next release stating that admin_menu will be removed in favour of navbar in a future release. Then, the upgrade notes for this future release could include the steps to cleanly switch over to navbar (uninstall admin_menu manually, and install navbar). Those that follow the release notes will get a clean removal of admin_menu. Those that don't will not. Then, the following release we actually remove admin_menu from the make file. The problem with that is that it requires people to upgrade sequentially. ie they would have to go from 1.1 -> 1.2 -> 1.3. If they went from 1.1 -> 1.3 skipping 1.2 they would not get admin_menu uninstalled properly and would be left with its cache tables and system variables hanging around.

Yeah, it's tricky. :-)

Since this isn't something we do very often and admin_menu is pretty simple, we could write code specifically to clean-up after admin_menu and update the status on the system table (there's a performance issue if you have a module enabled on the system table but it isn't present on disk: Drupal will scan the disk for it every request doing lots of futile stat() calls).

But we'd have to be sure to only do it if the user only has admin_menu in profiles/panopoly/modules/contrib and not in sites/all/modules or sites/*/modules (in the case of multi-site). Then there's also modules that depend on admin_menu, for example: Admin Menu Source, Adminimal Menu, etc. We'd have to make sure to disable those as well (although, we could leave uninstall to the user).

The lucky thing is that this is only part of panopoly.info! Those dependencies (admin, admin_menu) aren't in any panopoy_*.info modules, so this wouldn't affect any distributions based on Panopoly, only people who directly installed the Panopoly profile itself. If we had to worry about other distributions, this would get much trickier!

Edit: Sorry, I was thinking crazy on that paragraph. :-) The admin/admin_menu modules AREN'T in panopoly.info.

So, in conclusion, I think we could conceivably do admin/admin_menu this way, if we do it very very carefully, but this isn't really a general solution. More complex modules (I have "defaultcontent" in mind) really should be cleaned up by the module itself AND if the user is using it for something outside of Panopoly, we can't just plain disable it. I'm really not sure how to handle that.

We're going need to spend some more time thinking, experimenting and discussing before we can take action on removing things from .make files.

Comment #17 is sounding like the most reasonable option so far, assuming we can stop the update process and deliver a good message. We need to try it and see what it looks like, but it's probably doable!

Actually, you know what would be cool as well: a hook_requirements() which shows a warning/errror for every module that is enabled on the system table, but not found on disk. So, if they missed the upgrade message (or just plain didn't do it - some users forget) then this would be a constant reminder. Like I mentioned in comment #16, this causes performance issues too, so bugging the user to look into it would be good in general. (Unless that hook_requirements() is being run every request too, but I don't thing it is - I think it's cached). This would also need some experimentation. :-)