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

[11.x] Update app.php config stub to match default config file #51729

Closed
wants to merge 1 commit into from

Conversation

MaxGiting
Copy link
Contributor

I was getting errors when trying to use the new dontMergeFrameworkConfiguration() setting.

It was because when I published all the config files the stub file for app.php was missing a few array keys.

Should these stub files always match the base config files?

@crynobone
Copy link
Member

was getting errors when trying to use the new dontMergeFrameworkConfiguration() setting.

Shouldn't your config/app.php contain all the possible options when you wish to use dontMergeFrameworkConfiguration()?

Copy link
Member

@crynobone crynobone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please create a bug report with a reproducing repository or add integration tests to verify this issue.

@MaxGiting
Copy link
Contributor Author

MaxGiting commented Jun 7, 2024

Shouldn't your config/app.php contain all the possible options when you wish to use dontMergeFrameworkConfiguration()?

Yes that is correct. config/app.php needs to contain all keys, but when running php artisan config:publish --all --force that is not what happens. The published config file comes from config-stubs/app.php which is missing some used keys.

I guess my question really is. Should the config-stubs/app.php contain all the default and required keys that config/app.php has?

@MaxGiting
Copy link
Contributor Author

@crynobone
As suggested I have created a bug report and related example repo.
#51736

@crynobone
Copy link
Member

Seems it would make more sense to publish from config instead of config-stubs when using don't merge framework configuration.

@crynobone
Copy link
Member

Submit an alternative PR #51751

@taylorotwell
Copy link
Member

Closed in light of #51751

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.

3 participants