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.

Sign up for GitHub

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

Jump to bottom

Test auth is binded before detect Auth driver #946

Merged
merged 2 commits into from
Jun 6, 2020
Merged

Test auth is binded before detect Auth driver #946

merged 2 commits into from
Jun 6, 2020

Conversation

zhwei
Copy link
Contributor

@zhwei zhwei commented May 29, 2020

some project may not need Auth service provider

#649

zhwei added 2 commits May 29, 2020 20:52
@zhwei
Test auth is binded before detect Auth driver
353d3a3 
some project may not need Auth service provider
@zhwei
fix codingstyle
bcbc5d1
@mfn
Copy link
Collaborator

mfn commented May 31, 2020

How do you produce such a scenario when it's always bound in \Illuminate\Foundation\Application::registerCoreContainerAliases? Is it a project using separate components?

Just curious…

@zhwei
Copy link
Contributor Author

zhwei commented Jun 3, 2020

@mfn Aliases created in registerCoreContainerAliases is \Illuminate\Auth\AuthManager::class
link to auth, auth is bind in \Illuminate\Auth\AuthServiceProvider::registerAuthenticator

so if unregister AuthServiceProvider, bound('auth') will be false.

@zhwei
Copy link
Contributor Author

zhwei commented Jun 5, 2020

anyone can review this PR?

mfn
mfn approved these changes Jun 6, 2020
View reviewed changes
Copy link
Collaborator

@mfn mfn left a comment

Choose a reason for hiding this comment

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

LGTM 👍

ping @barryvdh

@mfn
Copy link
Collaborator

mfn commented Jun 6, 2020

Thanks for the details, much appreciated 🙇

@barryvdh barryvdh merged commit f3e48cb into barryvdh:master Jun 6, 2020
@zhwei zhwei deleted the patch-1 branch June 8, 2020 06:33
@mfn mfn mentioned this pull request Jun 26, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mfn mfn mfn approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@zhwei @mfn @barryvdh