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

Merge config file in the service registration #90

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

Conversation

ElfSundae
Copy link

In this PR, I simply renamed the boot to register, and deleted boot as it is unused for now.

mergeConfigFrom should be called in the register method, it means once the package being registered, its config are ready to use. Then other services can access them safely without caring about the order of application services bootstrap.

This is also mentioned in the official documentation:

To merge the configurations, use the mergeConfigFrom method within your service provider's register method.

publishes is useful only in the console application, and in the console application all services will be booted, so it does not matter putting publishes whether in register or boot method. Some core services also put it in register to make code organized, such as MailServiceProvider::registerMarkdownRenderer.

@fideloper
Copy link
Owner

I'll test it out but sounds good, appreciate it!

@kargnas
Copy link

kargnas commented Dec 4, 2017

Doesn't merged yet? I think this should be merged.

@ElfSundae
Copy link
Author

😅

@fideloper
Copy link
Owner

fideloper commented Dec 4, 2017

Have a new born at home, things will be slow to change :D

If you can help test this on various versions of Laravel, that'd be great. This package is still meant to be backwards compatible for all Laravel 5.* if possible.

@ElfSundae
Copy link
Author

@fideloper 😀 Congrats 🎉

@fideloper
Copy link
Owner

fideloper commented Dec 4, 2017 via email

@GrahamCampbell
Copy link
Contributor

👎 I'd dispute the recommendations in the docs actually. Makes much more sense to apply the config fallbacks during boot, since register shouldn't assume the config system is ready yet.

eg https://github.com/GrahamCampbell/Laravel-GitHub/blob/master/src/GitHubServiceProvider.php

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.

4 participants