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

[8.x] Replace direct application class mention by illuminate contract #36089

Conversation

alexmakii
Copy link
Contributor

This PR replaces Illuminate\Foundation\Application by contract Illuminate\Contracts\Foundation\Application.

Illuminate\Foundation\Application was accidentally added by #35204.

Reasons:
As illuminate/database can be used outside of the Laravel framework (with Lumen in my case) - direct mention of Laravel only classes should be avoided when it's possible.

@alexmakii alexmakii force-pushed the replace-outdated-interface-by-contract branch from 0cf62f1 to c3328a6 Compare January 29, 2021 13:56
@alexmakii
Copy link
Contributor Author

@taylorotwell Can you you explain why this PR was closed ?

@BrandonSurowiec
Copy link
Contributor

BrandonSurowiec commented Jan 29, 2021

@alexmakii One reason would be that the tests didn't pass:

There were 2 errors:

1) Illuminate\Tests\Database\DatabaseEloquentFactoryTest::test_resolve_nested_model_factories
Use of undefined constant Container - assumed 'Container' (this will throw an Error in a future version of PHP)

/home/runner/work/framework/framework/src/Illuminate/Database/Eloquent/Factories/Factory.php:713
/home/runner/work/framework/framework/src/Illuminate/Database/Eloquent/Factories/Factory.php:722
/home/runner/work/framework/framework/tests/Database/DatabaseEloquentFactoryTest.php:424

2) Illuminate\Tests\Database\DatabaseEloquentFactoryTest::test_resolve_non_app_nested_model_factories
Use of undefined constant Container - assumed 'Container' (this will throw an Error in a future version of PHP)

/home/runner/work/framework/framework/src/Illuminate/Database/Eloquent/Factories/Factory.php:713
/home/runner/work/framework/framework/src/Illuminate/Database/Eloquent/Factories/Factory.php:722
/home/runner/work/framework/framework/tests/Database/DatabaseEloquentFactoryTest.php:443

@taylorotwell
Copy link
Member

@alexmakii Literally had code that was invalid PHP in it - meaning you didn't test it even by running it.

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