Needs work
Project:
Media entity
Version:
8.x-2.0-alpha2
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
5 Jan 2018 at 09:26 UTC
Updated:
25 Jan 2018 at 02:07 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
berdirThe 8.x-2.x branch only exists to update an existing installation. You should not install the module in that version anywhere, use the core media module instead.
That said, yes, no point in keeping that function around. Or maybe throw an exception, but it's already too late at that point, can we use an install-time requirement maybe?
Comment #3
hussainweb@Berdir, that makes sense, however, installing Lightning fails right now because Lightning depends on this module (even if Drupal 8.4 is installed). Do you have any suggestions for handling this?
Comment #4
berdirThat sounds like a bug in lightning, you should open an issue there.
Comment #5
hussainwebThis patch should fix the problem. Seeing that we can't avoid installing this module in some scenarios, please consider committing.
Comment #6
hussainweb@Berdir, I agree. I only think it is this way to provide a transition. Still, this could be "fixed" in some way.
Comment #7
berdirI don't think this goes into the right direction. lightning needs to add the module for the upgrade path but the installer should not rely on it and the module should uninstall itself.
Media Entity 8.x-2.x must not be installed and we should explicitly prevent users from doing that, not fixing it.
Comment #8
hussainweb@Berdir, I see. Thanks for responding.
Comment #9
nikunjkotecha@Berdir there are cases were site is installed in automated scripts (travis build / deployment scripts). For such cases, it is important to make sure install of module works without a FATAL error.
Just a suggestion - we can have nothing in hook_install() of the module considering the fact that it is for update only but having code which throws FATAL error is not good IMO.
Comment #10
berdirautomated scripts do whatever you tell them to. media_entity 8.x-2.x should not be installed, it only exists for the upgrade path.
This was discussed a bit today in slack #media and the summary was that we should add a hook_requirments() that disallows to install completely.
Comment #11
jorgensean commentedI agree with @nikunjkotecha, we shouldn't be breaking installable state ever, even if the module is never meant to be installed stand-alone again. Automated scripts that make sure the system is in an installable state will trip here. Having a non-existing function in a module that "only exists for the upgrade path" isn't very helpful either.
Rerolled patch #5 against update_8201 which moved settings.