Skip to content

Use the ::class notation #3405

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

Merged
merged 1 commit into from
Jun 5, 2015
Merged

Use the ::class notation #3405

merged 1 commit into from
Jun 5, 2015

Conversation

nWidart
Copy link
Contributor

@nWidart nWidart commented Jun 5, 2015

Use the ::class notation for class references in app/ folder.
Similar to #3403

Prefix the namespace with \
taylorotwell added a commit that referenced this pull request Jun 5, 2015
@taylorotwell taylorotwell merged commit e4796a2 into laravel:develop Jun 5, 2015
@RomainLanz
Copy link

Do we need the slash at the beginning of each line?

@taylorotwell
Copy link
Member

Yes since we're in a namespace.

@RomainLanz
Copy link

Correct. Thanks for this answer.

@nWidart nWidart deleted the patch-1 branch June 5, 2015 13:55
@@ -13,7 +13,7 @@ class Kernel extends ConsoleKernel
* @var array
*/
protected $commands = [
'App\Console\Commands\Inspire',
\App\Console\Commands\Inspire::class,
Copy link
Member

Choose a reason for hiding this comment

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

Prefer if we imported this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What if we have 30 commands ? 😃
That was your argument on the other PR, not having tons of imports at top of file.

@rtablada
Copy link

rtablada commented Jun 5, 2015

Local app ::class will be relative or you could import the namespace (I know namespace imports in PHP aren't ideal). So that brings it down to a shorter length and a single import.

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.

5 participants