[FrameworkBundle] Added about command #19278
Conversation
| Q | A |
|---|---|
| Branch? | "master" |
| Bug fix? | no |
| New feature? | yes |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | comma-separated list of tickets fixed by the PR, if any |
| License | MIT |
| Doc PR | reference to the documentation PR, if any |
|
Could additionally also display the cache and log dirs here. |
|
Nice one! |
|
|
|
See the fabbot.io patch. |
|
Nah it's the inline |
| return new \DateTime() > \DateTime::createFromFormat('m/Y', $date); | ||
| $date = \DateTime::createFromFormat('m/Y', $date); | ||
| $date->modify('last day of this month'); | ||
| $date->setTime(23, 59, 59); |
There was a problem hiding this comment.
This works too: last day of this month 23:59:59
|
What if you try |
|
Last thing i could think of is adding some PHP / caching / platform info.. but other then that.. i like it :) |
|
Nice improvement! Can you add tests for this new command? |
|
Before reviewing the code, I'm wondering which problems it solves? What's the added value? Symfony already has quite a few built-in commands, so adding a new one should be considered carefully, it should add some value. Here, why and when would I use this command? |
|
I would mainly use this command to have a "CLI WDT" for applications that do not have pages that are able display the WDT; APIs and Command Line related (e.g. cron). Additionally, options could be passed along to only display a certain setting. This way you can easily collect a lot of info of multiple applications at once. This is great when trying to streamline applications. As my company has at least 15 applications running at all times, it would make finding inconsistencies between kernels easier. It would also help detect which bundles we use in which environment and easily compare without having to check different source files. Just my 2 cents of why I'd like to have this feature. |
|
For me; i want to leave the CLI as little as possible. Not want to dive into code / visit websites to get some (version) specifications. Also consider a environment where colleagues have their own workingcopy, docker container etc. I want to debug/review/troubleshoot their setup in a blink of an eye. Besides that, it's definitely nice to have, which always counts for value imo. edit: @iltar even the WDT doesnt expose all information like EOL, EOM etc. ;-) Ie. i would also like to see this info from there (different story ;-)). |
|
@ro0NL I don't realy see the value of this command in the framework. How about make a Bundle to share this command ? If you have a lot of Symfony websites to manage, could be nice to return theses useful informations into a dashboard :) What do you think ? |
|
A whole bundle for 1 command is a bit much :) but i'm really not a package maintainer or so. If it's not accepted, and you like, it's here to take 👍 Anyway, why it should be accepted :) These are framework specs, numbers, etc., hence the framework should also expose them, preferably from the CLI (imo). But yes, also from the WDT as some specs may differ (however that's not what this PR is about). Basicly it's about, is the info useful enough yes/no. I would say yes, (for me) especially when entering someone else's application (work, opensource "editions", my environment, docker environments, etc.) to get a quick scan of whats going on. I.e. big 👍 for me if i can do this in whatever symfony (full-stack) application. We could argue this needs a dedicated command vs. |
| /** | ||
| * @var string | ||
| */ | ||
| private $baseDir; |
There was a problem hiding this comment.
I don't think this prop is legitimate. Looking at the code, you should inject the value as an argument to formatPath instead
|
Maybe we should explicitly split symfony specs from kernel specs (although code-wise it's all kernel related, stuff like
ping @stof |
|
The bundle list looks really chaotic, have you tried what it would look like if you'd introduce a 3rd column for it? |
|
Not sure =/
We could also do |
|
I would be 👍 for the option, it's a bit that won't render well on small screens |
|
We can also leave the path out, and keep it a simple list. Perhaps later someone can enhance it with detailed bundle information. |
|
Excellent command. What do you think about adding a third horizontal separator for the directories? You have 'Symfony', 'Kernel', what about 'Directories'? |
|
Not sure.. the directories are kernel metadata. The 2 lines separating directories and bundles was just my own creativity 👼 to create some form of grouping (of kernel metadata). |
|
Final thoughts; Goal of this PR is to bring (more) parts of the WDT to the console. Why?
How?
Worth it? |
|
One thing I often notice people having trouble with, is that Apache and CLI use a different php.ini, perhaps that could be listed in both the WDT and CLI? |
|
You mean to reference the path to php.ini somewhere? |
|
Updated to
|
|
Reading the output of the command, it looks like it's more about the config than debug, right? |
|
More or less..., i guess its main purpose is to debug while it's actually showing config. What do you propose? I just had this wild idea of a global about command.. |
|
Indeed, I like the |
|
I like |
|
Done. Love it. Current command description is |
|
Also playing around with adding some git info (reading straight from
|
|
@ro0NL I don't have an opinion about your last proposal ... but I recommend you to not add any other feature to the command. Otherwise we won't be able to stabilize it on time for Symfony 3.2. |
|
Agree. I wouldnt mind though if this becomes a longer table list in the future (i like specs :)), but we should consider each entry carefully. Last but not least, im considering to remove the bundle paths. Not sure how useful those actually are 😕 it would cleanup the output a lot :) It could be even a comma separated value perhaps, in case of many bundles. |
|
I would suggest to add "Base directory" without Git info, but renamed to "Root directory" which looks more like the "App directory". |
|
Root is relative to the kernel group, ie. it reflects Also reconsidered adding the base directory, you probably know where your project lives already. |
| { | ||
| $this | ||
| ->setName('about') | ||
| ->setDescription('Displays information about the current installation') |
| $baseDir = realpath($kernel->getRootDir().DIRECTORY_SEPARATOR.'..'); | ||
| $bundles = array_map(function ($bundle) use ($baseDir) { | ||
| return '* '.$bundle->getName().' (<comment>'.self::formatPath($bundle->getPath(), $baseDir).'</>)'; | ||
| }, $kernel->getBundles()); |
There was a problem hiding this comment.
I would not list the bundles as there are easily accessible in the Kernel class.
| array('Timezone', date_default_timezone_get().' (<comment>'.(new \DateTime())->format(\DateTime::W3C).'</>)'), | ||
| array('OPcache', extension_loaded('Zend OPcache') && ini_get('opcache.enable') ? 'true' : 'false'), | ||
| array('APCu', extension_loaded('apcu') && ini_get('apc.enabled') ? 'true' : 'false'), | ||
| array('Xdebug', extension_loaded('xdebug') ? 'true' : 'false'), |
There was a problem hiding this comment.
Displaying this information can be misleading as it can be different from the PHP used by the web server. People should use php itself to get this information. I would restrict the information to what is really specific to Symfony.
There was a problem hiding this comment.
It shows the same info as WDT configuration panel does.. 🤔 which was the basic idea; to mimic this on CLI, so you can actually spot differences at first sight.
I dropped the bundle list (sounds reasonable), the version ID and an additional table separator.
I think it looks pretty clean :))
|
Thank you @ro0NL. |
This PR was merged into the 3.3-dev branch. Discussion ---------- [FrameworkBundle] Added about command | Q | A | | --- | --- | | Branch? | "master" | | Bug fix? | no | | New feature? | yes | | BC breaks? | no | | Deprecations? | no | | Tests pass? | yes | | Fixed tickets | comma-separated list of tickets fixed by the PR, if any | | License | MIT | | Doc PR | reference to the documentation PR, if any |  Commits ------- 2550eab [FrameworkBundle] Added about command
|
Awesome 💯 |