feat: add the EnvironmentDetector utility class and environmentdetector service#10130
feat: add the EnvironmentDetector utility class and environmentdetector service#10130paulbalandan wants to merge 6 commits intocodeigniter4:4.8from
EnvironmentDetector utility class and environmentdetector service#10130Conversation
michalsn
left a comment
There was a problem hiding this comment.
I think it's a good change.
I added some suggestions to the phpdocs / user guide.
7231481 to
86fe9de
Compare
| public function __construct(?string $environment = null) | ||
| { | ||
| if ($environment !== null && trim($environment) === '') { | ||
| throw new InvalidArgumentException('Environment cannot be an empty string.'); | ||
| } | ||
|
|
||
| $this->environment = $environment !== null ? trim($environment) : ENVIRONMENT; | ||
| } |
There was a problem hiding this comment.
Maybe skip double trim()?
| public function __construct(?string $environment = null) | |
| { | |
| if ($environment !== null && trim($environment) === '') { | |
| throw new InvalidArgumentException('Environment cannot be an empty string.'); | |
| } | |
| $this->environment = $environment !== null ? trim($environment) : ENVIRONMENT; | |
| } | |
| public function __construct(?string $environment = null) | |
| { | |
| $environment = $environment !== null ? trim($environment) : ENVIRONMENT; | |
| if ($environment === '') { | |
| throw new InvalidArgumentException('Environment cannot be an empty string.'); | |
| } | |
| $this->environment = $environment; | |
| } |
|
|
||
| As an alternative to reading the ``ENVIRONMENT`` constant directly, CodeIgniter | ||
| provides the ``environmentdetector`` service, backed by the | ||
| :php:class:`CodeIgniter\\EnvironmentDetector` class. Because it is a shared |
There was a problem hiding this comment.
It seems that php:class is used to describe classes and provide links to their descriptions. In this case, it looks like an ordinary code mention and you can skip it.
In general, what was the primary point of describing classes and functions in documentation in a special style? It looks clumsy. If you follow the style, you need to manually perform the functions of phpDocumentor. Due to the possible migration from Sphinx, this has added to the problems.
| * mocking this service. Mocking it does not modify the `ENVIRONMENT` | ||
| * constant. It only affects code paths that resolve and use this service. | ||
| */ | ||
| public static function environmentdetector(?string $environment = null, bool $getShared = true): EnvironmentDetector |
There was a problem hiding this comment.
Add to system/Config/BaseService.php:
* @method static EnvironmentDetector environmentdetector(?string $environment = null, bool $getShared = true)There was a problem hiding this comment.
I would also shorten (19 > 11 chars) the name to envdetector, env_detector, although a clear description is more important.
There was a problem hiding this comment.
how about just environment? it would read more naturally as service('environment')->isProduction() etc.
the class name EnvironmentDetector was chosen because it may clash with the env command's class when importing in IDE.
There was a problem hiding this comment.
Then it might be worth adding everything related to ENV to the service. And it will replace or complement the built-in DotEnv... At the moment, the "detector" clearly indicates its role.
There was a problem hiding this comment.
Ok. I'll just rename to envdetector.
Description
This PR adds the
EnvironmentDetectorutility class as a simple mockable wrapper to accessing theENVIRONMENTconstant, allowing us to test previously unreachable code branches dependent on the current environment.Checklist: