Use yiisoft/yii-console with optional.#178
Conversation
terabytesoftw
commented
Nov 21, 2023
| Q | A |
|---|---|
| Is bugfix? | ❌ |
| New feature? | ✔️ |
| Breaks BC? | ❌ |
PR Summary
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #178 +/- ##
=========================================
Coverage 94.31% 94.31%
Complexity 340 340
=========================================
Files 40 40
Lines 915 915
=========================================
Hits 863 863
Misses 52 52 ☔ View full report in Codecov by Sentry. |
| } | ||
| }, | ||
| "bin": [ | ||
| "bin/queue" |
There was a problem hiding this comment.
| "bin/queue" | |
| "bin/yii-queue" |
It's global space, I suggest use prefix.
There was a problem hiding this comment.
The idea when removing the console is to rename the package to yiisoft/queue, so queue makes sense.
There was a problem hiding this comment.
I suggest rename utility only. Because it is global namespace
bin/queue
Outdated
| use Yiisoft\Di\Container; | ||
| use Yiisoft\Di\ContainerConfig; | ||
|
|
||
| require_once './vendor/autoload.php'; |
There was a problem hiding this comment.
Could there be a problem if run utility from different places?
May be better use absoulte path via __DIR__ (see yii2)?
There was a problem hiding this comment.
It is not necessary when using ./ it works well for commands, whether you install the package in an app or clone it.
There was a problem hiding this comment.
I try it, and this do not work correclty.
There was a problem hiding this comment.
As it fails, to reproduce it, since I use it in the app, and in the cloned directory, and it works correctly.
There was a problem hiding this comment.
But you are inside the config directory, you must be root :)
There was a problem hiding this comment.
Utlitiy should work from any directory. Just use absolute path.
bin/queue
Outdated
| use Yiisoft\Di\Container; | ||
| use Yiisoft\Di\ContainerConfig; | ||
|
|
||
| $autoload = $GLOBALS['_composer_autoload_path'] ?? './vendor/autoload.php'; |
There was a problem hiding this comment.
| $autoload = $GLOBALS['_composer_autoload_path'] ?? './vendor/autoload.php'; | |
| $autoload = $_composer_autoload_path ?? dirname(__DIR__, 3) . '/vendor/autoload.php'; |
bin/queue
Outdated
| require_once $autoload; | ||
|
|
||
| /** @var array $definitions */ | ||
| $definitions = require_once 'definitions.php'; |
There was a problem hiding this comment.
| $definitions = require_once 'definitions.php'; | |
| $definitions = require_once __DIR__ . 'definitions.php'; |
There was a problem hiding this comment.
If you use the absolute path, when you copy the definitions to the ROOT Directory of the app, not work.
There was a problem hiding this comment.
If necessary, suppose that i have a different configuration than the predetermined, for example, i used monolog, when updating new version will lose my changes, that is fine, only when your configuration is equal to the predetermine
README.md
Outdated
|
|
||
| Just install `yiisoft/yii-console` package and you are ready to go. | ||
|
|
||
| ## Usage with Symfony Console |
There was a problem hiding this comment.
For use with Symfony Console we should add commands to Symfony Console and do not use files from bin.
There was a problem hiding this comment.
https://symfony.com/doc/current/components/console.html#creating-a-console-application Well in the documents it clearly says that you must register a script and add the commands, it is what is done in this PR.
There was a problem hiding this comment.
We should add instruction for use commands with symfony console that alreasy used in user app.
There was a problem hiding this comment.
I have to investigate it, i suggest merging this PR, and do it in another.
There was a problem hiding this comment.
https://symfony.com/doc/current/console.html#registering-the-command According to this they are already registered, and the Symfony console should recognize them.
|
yiisoft/actions#66 fix mutation actions. |
|
|
||
| ## Usage with Queue utility | ||
|
|
||
| 1. Copy configuration file `./vendor/yiisoft/yii-queue/bin/QueueContainer.php` to `root` folder of your project. |
There was a problem hiding this comment.
Suggest require the user to create yii-queue-params.php in root directory with queue params only and don't require copy full container configuration. It's more cleary...
| @@ -0,0 +1,83 @@ | |||
| <?php | |||
There was a problem hiding this comment.
Emm, why do we need it?
Isn't it better to use all users' configs instead?
There was a problem hiding this comment.
Or even better, use a ServiceProvider here (because the signature is the same)
|
I think we need to adjust it the same way as yiisoft/db-migration@e0e4e49 |
|
@terabytesoftw is it possible to resolve conflicts? |


