Move checks to separate commands/Refactoring

[steverobbins]

When I started this my only goal was to try and make a Symphony Console app with play with travis/phpunit. To keep this going I think some refactoring is needed. As part of that initiative I think it's best to separate out the commands.

The ScanCommand class is growing more complex and will continue to grow with each new feature. Before long it won't be maintainable. I'm not certain on the design yet, but here is my proposal:

# list available checks
magescan list
# check magento version, etc
magescan version store.example.com
# run all checks
magescan all store.example.com

For the list command to work, each command would need to be identified, perhaps the way n98-magerun does it with a config.yaml. I guess this means I need to customize the Application object as well.

It would also be nice to setup some sort of ~/.magescan/config.ext to locally set things like HTTPS validation (#65).

In the long run I think this will make the app more sustainable, easier to develop, and will lead to better unit testing.

[pocallaghan]

Your application already supports the list command as it's a default feature of an Application. It will list all the commands you add (in your case currently the scan command added in bootstrap) :). The main advantage of the config approach is it allows plugins to add commands without editing the contents of the phar.

I'm definitely behind the idea of adding some kind of config. It would be cool to add in your own paths to the UnreachablePaths check and also modules into the Module check.

One thing I'm not a big fan of in n98-magerun is the fact that most of the business logic is performed in the Command itself. I like the fact that the logic for your Checks are (mostly) not performed in the command but in a separate class. This maintains the controller > model paradigm better IMO and makes for easier 'true' unit testing.

[steverobbins]

Good point. There's already a list command.

For sensitive paths/modules I was thinking a csv/url-per-line file could be passed as an argument. It would be cool to set it once in a config too (i.e. .magescan/additional-unreachable-paths.txt).

And yeah, I started moving the check logic out once I knew I wanted to build a web interface. There's still work to do though.