Robo\Result::error() first argument not clear

connorshea commented 5 years ago

Steps to reproduce

I have a file like so:

<?php
namespace SuiteCRM\Custom\Robo\Plugin\Commands;
use Robo\Task\Base\loadTasks;

class CustomCommands extends \Robo\Tasks
{
    use loadTasks;

    public function deploy()
    {
        $currentShell = $this->taskExec('echo "$SHELL"')->printOutput(false)->run();

        if ($currentShell->getMessage() !== "/bin/bash") {
             return \Robo\Result::error($this, "You must use bash to run this script.");
        }

        # Do other stuff
    }
}

I'd like to fail it when the shell running the task isn't bash (I'm using Windows and it seems like it was trying to run in cmd).

Expected behavior

I expected the script to print "You must use bash to run this script." when I ran it from a shell that wasn't bash, or to continue with the task otherwise.

Actual behavior

I get a failure that looks like this:

$ ./vendor/bin/robo deploy 
[Exec] Running echo "$SHELL"
[Exec] Done in 0.09s
PHP Fatal error:  Uncaught TypeError: Argument 1 passed to Robo\Result::error() must implement interface Robo\Contract\TaskInterface, instance of SuiteCRM\Custom\Robo\Plugin\Commands\CustomCommands given, called in /cygdrive/c/inetpub/msm_d ev/custom/lib/Robo/Plugin/Commands/CustomCommands.php on line 22 and defined in /cygdrive/c/inetpub/msm_dev/vendor/consolidation/robo/src/Result.php:114

The documentation doesn't say what I'm supposed to be passing into the Robo\Result::error() method and I'm not entirely clear on how I'd take $this and turn it into an object implementing TaskInterface, like it wants.

System Configuration

Which O.S. and PHP version are you using?

OS: Windows 7 PHP: PHP 7.1.12 (cli) (built: Dec 7 2017 02:53:41) ( NTS ) Robo: 1.3.2

connorshea commented 5 years ago

I can do it like this:

throw new \RuntimeException("You must use bash to run this script.

But if the comments are to be believed, it's not how it's supposed to be done ideally.

greg-1-anderson commented 5 years ago

The comment in the examples is indeed not clear on this point.

Result objects are returned by tasks. The Robo model is that all operations should be implemented in tasks, and Robo commands should simply make a task collection, run it, and return the result.

If you wish to exit early from a command, the best thing to do is just throw an exception.

Also, it is not necessary for you to use taskExec if you do not need to. In your example above, taskExec reports that it is running echo $SHELL and then it reports how long it took. If you don't want this, you can simply call PHP's exec, or use the Symfony Process component.

connorshea commented 5 years ago

@greg-1-anderson Thanks! Still new to PHP, I'll take a look at PHP's exec, it's probably better suited for what I want to do here :)

greg-1-anderson commented 5 years ago

consolidation / robo