Skip to content

Update providers and alises for 5.1 consistency #80

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
Aug 24, 2015

Conversation

josezenem
Copy link
Contributor

In Laravel 5.1 they use class name resolution via ::class, this is just to make it consistent with the rest of the config/app.php.

In Laravel 5.1 they use class name resolution via ::class, this is just to make it consistent with the rest of the `config/app.php`.
jeskew added a commit that referenced this pull request Aug 24, 2015
Update providers and alises for 5.1 consistency
@jeskew jeskew merged commit 02adcec into aws:master Aug 24, 2015
@GrahamCampbell
Copy link
Contributor

👎

@GrahamCampbell
Copy link
Contributor

Don't see why this needed to change.

@josezenem
Copy link
Contributor Author

For consistency with of 5.1, not sure why you wouldn't want it to change. Everything is using the class ::class annotation now. http://laravel.com/docs/5.1/providers#registering-providers

The package for 5.0 https://github.com/aws/aws-sdk-php-laravel/tree/2.0 and 4 series https://github.com/aws/aws-sdk-php-laravel/tree/1.0 keep the non ::class annotation which matches with their documentation.

@GrahamCampbell
Copy link
Contributor

not sure why you wouldn't want it to change

Because most people don't use the ::class notation in their config files because it's totally pointless.

@GrahamCampbell
Copy link
Contributor

It's only useful when you want to reference classes in your own code and have an IDE capable of working out what to do, say if you want to refactor.

@josezenem
Copy link
Contributor Author

You make some valid points. However the change is to stay consistent with Laravel 5.1 style and documentation, not address what users may or may not be doing.

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