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

[make:twig-component] read processed twig_component config instead of parsing the yaml file #1618

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

IndraGunawan
Copy link
Contributor

changes from #1571 breaks if config is in php files even $namespace has a default value

this PR read the processed twig_component config no matter how the config was created

@IndraGunawan IndraGunawan changed the title read processed twig_component config instead parse the yaml file [make:twig-component] read processed twig_component config instead parse the yaml file Dec 5, 2024
@IndraGunawan IndraGunawan force-pushed the read-processed-config branch from 195e099 to b07a50d Compare December 5, 2024 11:50
@IndraGunawan IndraGunawan force-pushed the read-processed-config branch from b07a50d to 414f39b Compare December 5, 2024 11:56
@IndraGunawan IndraGunawan changed the title [make:twig-component] read processed twig_component config instead parse the yaml file [make:twig-component] read processed twig_component config instead of parsing the yaml file Dec 7, 2024
@smnandre
Copy link
Member

This will need to be updated if we change anything in the extension, add a pass, etc etc..

Seems a bit risky, but in the same time in understand the need 🤷

@IndraGunawan
Copy link
Contributor Author

@smnandre thanks for your feedback

if this change is a bit risky, i would revert the changes and instead make the command not throwing an error if the yaml config is not exist but it makes the feature only be available on yaml config

in current condition it is totally broken for php config

wdyt?

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

Successfully merging this pull request may close these issues.

2 participants