Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore remove statements in XML files #34

Open
norgeindian opened this issue Oct 13, 2022 · 2 comments
Open

Ignore remove statements in XML files #34

norgeindian opened this issue Oct 13, 2022 · 2 comments

Comments

@norgeindian
Copy link

It's amazing, that XML files are now also checked for dependencies.
Nevertheless, I would say, that a statement like the following would not break anything, if the original module / file does not exist:

<remove src="Vendor_Module::css/module.css"/>

This currently leads to needed dependency on this module and could be removed in my eyes.

@jissereitsma
Copy link
Contributor

Good point. This actually opens up for a wider discussion. For instance, if a file etc/adminhtml/system.xml is there, it would a suggest a dependency with Magento_Backend. But if, in some parallel universe, that backend would not be there, the file would simply not be picked up upon. Likewise with XML layout for a specific handle: If the responsible module is not there, the handle is never activated.

I actually am not sure how we should deal with this. Any idea? Also cc @sprankhub

@sprankhub
Copy link
Contributor

in some parallel universe

I think this is the main point here :) I'd argue that it is a valid, real-life use case to remove resources from modules that might not be installed (as shown in the issue). However, having the backend removed from Magento is just not very realistic. IMHO, it really depends on the concrete case.

Actually, the Hyvä detection could also be discussed:

$patterns = ['hyva_modal' => 'Hyva_Theme'];

It might be that hyva_modal is used for Hyvä themes, but for Luma-based themes, the module still works without it 🤔 I am fine with keeping it for now. If we encounter more such cases, it might be an idea to have two different types: "missing dependency" (error) and "possible missing dependency" / "suggested dependency" (warning).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants