Implement a factory method

[tormjens]

I am trying to build a package that more tightly integrates with this package, but with the restrictive nature of how this project is coded I'm not able to grab anything from.

This PR introduces a new public factory method on the FirebaseProject class.

This way one can use the factory built by this package to implement other FIrebase services not supported in the core of this package.

[codecov[bot]]

Codecov Report

Merging #76 (c2b1804) into main (84a0289) will decrease coverage by 1.55%. The diff coverage is 0.00%.

@@              Coverage Diff              @@
##                main      #76      +/-   ##
=============================================
- Coverage     100.00%   98.44%   -1.56%     
- Complexity        46       47       +1     
=============================================
  Files              3        3              
  Lines            127      129       +2     
=============================================
  Hits             127      127              
- Misses             0        2       +2     

Impacted Files	Coverage Δ	Complexity Δ
src/FirebaseProject.php	93.75% <0.00%> (-6.25%)	16.00 <1.00> (+1.00)	:arrow_down:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 84a0289...d121f60. Read the comment docs.

[jeromegamez]

While I was thinking about how you might want to integrate with this package it came to me that it's not meant to be integrated with 😅 - it's purpose is to provide a layer over the underlying SDK to make it configurable via Laravel's config files and accessible through Facades and Dependency Injection.

• The Laravel container is injected into the FirebaseProjectManager
• The FirebaseProjectManager uses the Laravel config to configure the factory and, with it, a FirebaseProject
• The FirebaseProject uses the Factory (internally) to initialize the single components

Here, the Factory is just an internal implementation detail, and I see no benefit from the package's perspective, to expose it. As that's the only way the factory is intended to be used in the scope of this package, I'm very hesitant to expose it via the FirebaseProject or at all 🙈 . I don't know how you feel about it, but for me it's kinda gross, reaching so deep into the internals of the package to reach one of the building blocks 😅.

Again, I don't know what you want to build that needs access to the factory, but if it's something that this package should have (e.g. a missing SDK component that's not exposed here), I would be happy to discuss a possible PR.

If it's a missing component/feature of the underlying SDK, let's discuss this on https://github.com/kreait/firebase-php/discussions so that it can possibly be implemented there and then made accessible here.

If it's an additional feature not in the scope of the Firebase ecosystem (e.g. you want to use the Factory::createApiClient() method, I would suggest a package that's independent of the classes in this one and implement the Factory initialization separately. You could still re-use the firebase config and/or environment variables this package provides, but you wouldn't need to depend on this package's classes.

Please know that I don't want to be rude, and I do realize that adding just this method could be considered as "not as big of a deal", but I create and maintain my projects with what I think works best (for me) and in my personal experience, opening up more than I need to most often lead to more problems and more work.

And please do let me know what you think, my opinions are never set in stone, I just need to be convinced 😅

[tormjens]

Yeah, I figured as much. And my PR was merely a suggestion. I understand that this package is a solution, more than a foundation, but some things are still somewhat open-ended.

My integration specifically is one towards Firestore without that PHP extension. While I can set up everything manually, I would assume many of us are using your package anyways.

I will probably just find a different way, but I thought I’d just give it a try.

Thanks for your work :)

tir. 26. jan. 2021 kl. 23:35 skrev Jérôme Gamez notifications@github.com:

While I was thinking about how you might want to integrate with this package it came to me that it's not meant to be integrated with 😅 - it's purpose is to provide a layer over the underlying SDK to make it configurable via Laravel's config files and accessible through Facades and Dependency Injection.

• The Laravel container is injected into the FirebaseProjectManager
• The FirebaseProjectManager uses the Laravel config to configure the factory and, with it, a FirebaseProject
• The FirebaseProject uses the Factory (internally) to initialize the single components

Here, the Factory is just an internal implementation detail, and I see no benefit from the package's perspective, to expose it. As that's the only way the factory is intended to be used in the scope of this package, I'm very hesitant to expose it via the FirebaseProject or at all 🙈 . I don't know how you feel about it, but for me it's kinda gross, reaching so deep into the internals of the package to reach one of the building blocks 😅.

Again, I don't know what you want to build that needs access to the factory, but if it's something that this package should have (e.g. a missing SDK component that's not exposed here), I would be happy to discuss a possible PR.

If it's a missing component/feature of the underlying SDK, let's discuss this on https://github.com/kreait/firebase-php/discussions so that it can possibly be implemented there and then made accessible here.

If it's an additional feature not in the scope of the Firebase ecosystem (e.g. you want to use the Factory::createApiClient() method https://github.com/kreait/firebase-php/blob/95a67de20d9ac77ea034fbcfd6cc6971556ea120/src/Firebase/Factory.php#L592, I would suggest a package that's independent of the classes in this one and implement the Factory initialization separately. You could still re-use the firebase config and/or environment variables this package provides, but you wouldn't need to depend on this package's classes.

Please know that I don't want to be rude, and I do realize that adding just this method could be considered as "not as big of a deal", but I create and maintain my projects with what I think works best (for me) and in my personal experience, opening up more than I need to most often lead to more problems and more work.

And please do let me know what you think, my opinions are never set in stone, I just need to be convinced 😅

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/kreait/laravel-firebase/pull/76#issuecomment-767872617, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA6KN2NKEXN7MY2ETRC4VCLS347U5ANCNFSM4WUBUBFA .