brefphp / bref

Serverless PHP on AWS Lambda
https://bref.sh
MIT License
3.09k stars 367 forks source link

Detect bref/secrets-loader installed as dependency of another package #1673

Closed kevincerro closed 10 months ago

kevincerro commented 10 months ago

If you require bref/secrets-loader in another package as dependency you get this warning on serverless cli Captura de pantalla 2023-10-22 a las 13 54 20

With this simple change we detect if bref/secrets-loader is really installed, as direct dependency or as a dependency of another package.

GrahamCampbell commented 10 months ago

Not sure we should allow transitive dependencies, as this can be risky. We should force applications to explicitly require it.

mnapoli commented 10 months ago

I agree with @GrahamCampbell, it's usually a best practice to explicitly require your direct dependencies.

Do you have a specific use case here? (I'm curious how the package is installed via another dependency, is it an internal package?)

kevincerro commented 10 months ago

I agree with @GrahamCampbell, it's usually a best practice to explicitly require your direct dependencies.

Do you have a specific use case here? (I'm curious how the package is installed via another dependency, is it an internal package?)

Yes, I made an internal php package for a laravel-based php framework to autoconfigure it for serverless using bref. I wanted to change less as possible the core, so creating an internal php package with all the needed was the way to go.

mnapoli commented 10 months ago

Makes sense.

The diff is very minimal and technically makes sense, I'm fine merging. Can you confirm you tested this and it works? (it's not covered by tests IIRC)

kevincerro commented 10 months ago

Makes sense.

The diff is very minimal and technically makes sense, I'm fine merging. Can you confirm you tested this and it works? (it's not covered by tests IIRC)

Yes, its working, warning is gone.

Before

Captura de pantalla 2023-10-23 a las 8 34 57

After

Captura de pantalla 2023-10-23 a las 8 36 33

mnapoli commented 10 months ago

Thanks!