feat: [Command] add options for spark optimize command

[warcooft]

Description

To make this command more flexible. This PR provides options to allow us to easily enable or disable cache configuration.

Usage

protected $usage = 'optimize [-c] [-l] [-d]';

Options

protected $options = [
    'c' => 'Enable config cache',
    'l' => 'Enable locator cache',
    'd' => 'Disable config and locator caching',
];

Example

// Enable configCache and locatorCache.
php spark optimize 

// Enable configCache only and ignore the others.
php spark optimize -c 

// Enable locatorCache only and ignore the others.
php spark optimize -l 

// Disable config and locator caching
php spark optimize -d

Checklist:

• Securely signed commits
• Component(s) with PHPDoc blocks, only if necessary or adds value
• Unit testing, with >80% coverage
• User guide updated
• Conforms to style guide

[paulbalandan]

I think it would be easier to understand the options if we use long options names, like --config-only.

[kenjis]

-r is also a bit difficult to understand. --disable or -d may be better?

[warcooft]

Using long options names i think would take extra time to write it. it's better if we keep it simple.
Or we can create additional options as aliases for each option?

[warcooft]

@kenjis -d I think is much better.

[kenjis]

Or we can create additional options as aliases for each option?

There is no commands that having a short and long options for one action.
So, use short options or use long options.

[kenjis]

What if you specify all options spark optimize -c -l -r?

[kenjis]

Please fix coding style, and add test code to prove this works fine.

[warcooft]

I don't have much experience with unit testing, so please let me know if it seems right.

[datamweb]

Good PR, every person to run actions.

[kenjis]

Why the method enableCaching() does disable caching?
It is strange and not intuitive.
Please add a new private method to disable caching.

[warcooft]

Done

[kenjis]

spark optimize -d does not reinstall dev packages.

[warcooft]

Can you guys run the workflow again. It seems everything is fine now.

[kenjis]

When I see the method name disableCaching(), I imagine the method disables caching.
But this method just returns an array.
Therefore the method name or the implementation is not good.

[warcooft]

i will send an update, this makes it more flexible.

[kenjis]

I don't understand what the runConfig() method dose when I see the method name.
Is updateConfigFile() better?

[warcooft]

When I see the method name disableCaching(), I imagine the method disables caching.
But this method just returns an array.
Therefore the method name or the implementation is not good.

If disabelCaching() and enableCaching() does its job completely, I don't think runCaching() is needed any more. we just need a process like this.

public function run(array $params)
    {
        // Parse options
        $enableConfigCache  = CLI::getOption('c');
        $enableLocatorCache = CLI::getOption('l');
        $disable            = CLI::getOption('d');

        try {
            if ($disable === true) {
                $this->disableCaching();
                $this->reinstallDevPackages();
            } else {
                $this->enableCaching(['config' => $enableConfigCache, 'locator' => $enableLocatorCache]);
                $this->removeDevPackages();
            }

            $this->clearCache();

        } catch (RuntimeException) {
            CLI::error('The "spark optimize" failed.');

            return EXIT_ERROR;
        }

        return EXIT_SUCCESS;
    }

[kenjis]

Good! That will be easier to understand.