Multiple projects #67
Conversation
Codecov Report
@@ Coverage Diff @@
## main #67 +/- ##
===========================================
Coverage 100.00% 100.00%
- Complexity 19 44 +25
===========================================
Files 1 3 +2
Lines 71 121 +50
===========================================
+ Hits 71 121 +50
Continue to review full report at Codecov.
|
src/FirebaseProject.php
Outdated
|
|
||
| class FirebaseProject | ||
| { | ||
| /** @var \Kreait\Firebase\Factory $factory */ |
There was a problem hiding this comment.
I usually don't include FQNs in PHPDocs, so this "risks" being changed to Kreait\Firebase\Factory (due to the existing import) or to Factory with an additional use statement somewhen in the future 🙈 (similarly elsewhere)
|
|
||
| class FirebaseProjectManager | ||
| { | ||
| /** @var \Illuminate\Contracts\Foundation\Application */ |
There was a problem hiding this comment.
Could you check if this equally works with Illuminate\Contracts\Container\Container? This would probably mean that getting the config would have to be implemented differently, but I remember changing Application to Container in the past for better Lumen support.
There was a problem hiding this comment.
I'll have a look into it. I changed it to copy the resolveCredentials method to FirebaseProjectManager, as basePath does not exist on Container.
src/ServiceProvider.php
Outdated
| @@ -5,6 +5,7 @@ | |||
| namespace Kreait\Laravel\Firebase; | |||
|
|
|||
| use Illuminate\Contracts\Container\Container; | |||
| use Illuminate\Contracts\Foundation\Application; | |||
There was a problem hiding this comment.
Same as in FirebaseProjectManager - I used Container in other parts of the app because of Lumen and if it's possible, we should stick with that
|
Could you please also remove support for Laravel/Illuminate 5.8 in the composer.json and GitHub actions? I think |
Co-authored-by: Jérôme Gamez <jerome@gamez.name>
Co-authored-by: Jérôme Gamez <jerome@gamez.name>
Co-authored-by: Jérôme Gamez <jerome@gamez.name>
|
I think I incorporated all your suggestions 😃
|
|
Sorry for the late response (again) - I just tested it locally and it works just fine, and using the new Facade is so much more enjoyable! I will merge this now, and after integrating the other open PR and making some minor changes, I'll release this as a new version! This will be a nice new major release, thank you so much! 🌺 |
This pull requests adds code to support multiple firebase projects in laravel via a new facade.
Since there is a breaking change in the configuration file, a major version bump is needed. Also, all existing facades are deprecated in favor of a single new facade to support multiple projects. They are however included for an easier upgrade path.