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

[5.4] Adjust app() to work with the new container changes #17060

Merged
merged 1 commit into from
Dec 31, 2016
Merged

[5.4] Adjust app() to work with the new container changes #17060

merged 1 commit into from
Dec 31, 2016

Conversation

KennedyTedesco
Copy link
Contributor

@taylorotwell Since resolve() is just an alias to app() maybe is time to stop providing it? If you do not think so, please let me know.

@GrahamCampbell
Copy link
Member

Please don't delete the resolve function. It's strictly not an alias, because it forces 1 param.

@KennedyTedesco
Copy link
Contributor Author

@GrahamCampbell Didn't see that way before. My bad. 😊 I have made some adjustments.

@KennedyTedesco
Copy link
Contributor Author

I know it's not strictly necessary, but I would change the resolve() to something like that:

if (! function_exists('resolve')) {
    /**
     * Resolve a service from the container.
     *
     * @param  string  $name
     * @return mixed
     */
    function resolve($name)
    {
        if(is_null($name)) {
            throw new InvalidArgumentException('You must provide the service name.');
        }

        return app($name);
    }
}

@taylorotwell taylorotwell merged commit d7d15f8 into laravel:5.4 Dec 31, 2016
@KennedyTedesco KennedyTedesco deleted the helpers-5.4 branch December 31, 2016 16:52
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