Feature/cli #308
Feature/cli #308
Conversation
* refactoring cli commands
iruzevic
added
improvement
Codecov Report
@@ Coverage Diff @@
## develop #308 +/- ##
=============================================
+ Coverage 81.12% 82.58% +1.46%
- Complexity 1195 1233 +38
=============================================
Files 131 130 -1
Lines 3602 3859 +257
=============================================
+ Hits 2922 3187 +265
+ Misses 680 672 -8
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Co-authored-by: Denis Žoljom <[email protected]>
Co-authored-by: Denis Žoljom <[email protected]> Co-authored-by: Davorin Prislin <[email protected]>
Co-authored-by: Igor Obradović <[email protected]>
Co-authored-by: Igor Obradović <[email protected]>
Co-authored-by: Igor Obradović <[email protected]>
Co-authored-by: Igor Obradović <[email protected]>
Co-authored-by: Igor Obradović <[email protected]>
Co-authored-by: Igor Obradović <[email protected]>
Co-authored-by: Igor Obradović <[email protected]>
Co-authored-by: Igor Obradović <[email protected]>
Co-authored-by: Igor Obradović <[email protected]>
Co-authored-by: Igor Obradović <[email protected]>
Co-authored-by: Igor Obradović <[email protected]>
| 'longdesc' => " | ||
| 'longdesc' => $this->prepareLongDesc(" | ||
| ## USAGE | ||
|
|
||
| This file is a main entrypoint for all our block editor setup. |
There was a problem hiding this comment.
| This file is a main entrypoint for all our block editor setup. | |
| This file is a main entry point for all our block editor setup. |
There was a problem hiding this comment.
"for all our block editor setup" also doesn't tell much, maybe rephrase that part as well
| class UseAssetsCli extends AbstractBlocksCli | ||
| { | ||
| /** | ||
| * Get WPCLI command parent name |
There was a problem hiding this comment.
| * Get WPCLI command parent name | |
| * Get WP-CLI command parent name |
| } | ||
|
|
||
| /** | ||
| * Get WPCLI command name |
There was a problem hiding this comment.
| * Get WPCLI command name | |
| * Get WP-CLI command name |
| } | ||
|
|
||
| /** | ||
| * Paths join |
There was a problem hiding this comment.
| * Paths join | |
| * Paths join. |
There was a problem hiding this comment.
| * Paths join | |
| * Join paths. |
#wordOrderMatters
| <?php | ||
|
|
||
| /** | ||
| * Class that registers WPCLI command initial setup for all. |
There was a problem hiding this comment.
| * Class that registers WPCLI command initial setup for all. | |
| * Class that registers WP-CLI command initial setup for all. |
| ]; | ||
|
|
||
| /** | ||
| * Get WPCLI command parent name. |
There was a problem hiding this comment.
| * Get WPCLI command parent name. | |
| * Get WP-CLI command parent name. |
| } | ||
|
|
||
| /** | ||
| * Get WPCLI command name. |
There was a problem hiding this comment.
| * Get WPCLI command name. | |
| * Get WP-CLI command name. |
| } | ||
|
|
||
| /** | ||
| * Get WPCLI command doc. |
There was a problem hiding this comment.
| * Get WPCLI command doc. | |
| * Get WP-CLI command doc. |
There was a problem hiding this comment.
Reviewed files changed, apart from tests, as it's been quite a task to review this. Congratulations, this was an amazing endeavor. I've left some comments about phrasing, and some tips for a potentially better approach in some places, but I don't take issue with this code. Great work.
| $sep = \DIRECTORY_SEPARATOR; | ||
| $pathName = self::PATH_WRAPPER; | ||
| $manifestPath = "{$this->getBlocksFolderPath()}{$pathName}{$sep}manifest.json"; | ||
| $path = Components::getProjectPaths('blocksDestinationWrapper', "manifest.json"); |
There was a problem hiding this comment.
Now I know this is a pet peeve, but let's be consistent 😄
| $path = Components::getProjectPaths('blocksDestinationWrapper', "manifest.json"); | |
| $path = Components::getProjectPaths('blocksDestinationWrapper', 'manifest.json'); |
|
|
||
| $root = $this->getProjectRootPath(); | ||
| $rootNode = $this->getFrontendLibsBlockPath(); | ||
| $isFile = \strpos($name, '.') !== false; |
There was a problem hiding this comment.
This is not a valid assumption to have on UNIX systems, e.g. /etc/hosts is a file. All files that'll be moved probably have an extension, but this is not a valid way to check whether something is a file.
Rather use is_file and pass the file path to it.
| self::cliError( | ||
| \sprintf( | ||
| // translators: %s will be replaced with type of item, and shorten cli path. | ||
| "%s files exist on this path: `%s`. If you want to override the destination folder please use --skip_existing='true' argument.", |
There was a problem hiding this comment.
Why does skip_existing=true imply you want to override the existing files? I would assume the opposite to be the case (skip_existing=false to override them, skip_existing=true to... well, skip them)
| $this->cliLog( | ||
| \sprintf( | ||
| // translators: %s will be replaced with type of item. | ||
| \esc_html__("We have found that this %s has dependencies, please run these commands also if you don't have it in your project:", 'eightshift-libs'), |
There was a problem hiding this comment.
| \esc_html__("We have found that this %s has dependencies, please run these commands also if you don't have it in your project:", 'eightshift-libs'), | |
| \esc_html__("This %s has dependencies. To resolve dependencies, if they aren't already resolved, run:", 'eightshift-libs'), |
| 'longdesc' => " | ||
| 'longdesc' => $this->prepareLongDesc(" | ||
| ## USAGE | ||
|
|
||
| This file is a main entrypoint for all our block editor setup. |
There was a problem hiding this comment.
"for all our block editor setup" also doesn't tell much, maybe rephrase that part as well
| $this->cliLog('We have moved everything we have to your project. Please type `npm start` in your terminal to kickstart your assets bundle process.', "M"); | ||
| $this->cliLog('Happy developing!', "M"); |
There was a problem hiding this comment.
Maybe something like this?
| $this->cliLog('We have moved everything we have to your project. Please type `npm start` in your terminal to kickstart your assets bundle process.', "M"); | |
| $this->cliLog('Happy developing!', "M"); | |
| $this->cliLog('It\'s done! All of the initial setup for Eightshift Development Kit has finished. Please run `npm start` in your terminal to kickstart your asset bundling process.', "M"); | |
| $this->cliLog('Happy development!', "M"); |
There was a problem hiding this comment.
I have created an issue for this #310
| if (!$groupOutput) { | ||
| $this->cliLog('--------------------------------------------------'); | ||
| $this->cliLog('We have moved everything you need to start creating WordPress blocks. Please type `npm start` in your terminal to kickstart your assets bundle process.', "M"); | ||
| $this->cliLog('Happy developing!', "M"); |
| if (!$groupOutput) { | ||
| $this->cliLog('--------------------------------------------------'); | ||
| $this->cliLog('We have moved everything you need to start creating your awesome WordPress project. Please type `npm start` in your terminal to kickstart your assets bundle process.', "M"); | ||
| $this->cliLog('Happy developing!', "M"); | ||
| } |
|
|
||
| if (!$groupOutput) { | ||
| $this->cliLog('We have moved everything you need to start creating your awesome WordPress theme. Please type `npm start` in your terminal to kickstart your assets bundle process.', "M"); | ||
| $this->cliLog('Happy developing!', "M"); | ||
| } | ||
| } | ||
| } |
Co-authored-by: Davorin Prislin <[email protected]>
Co-authored-by: Davorin Prislin <[email protected]>
Co-authored-by: Mario Borna Mjertan <[email protected]>
Co-authored-by: Mario Borna Mjertan <[email protected]>
Co-authored-by: Mario Borna Mjertan <[email protected]>
Co-authored-by: Mario Borna Mjertan <[email protected]>
Co-authored-by: Mario Borna Mjertan <[email protected]>
Description
Screenshots / Videos