[Question] Using type declaration in function definitions

[bsacharski]

Hello, just wanted to ask about function definitions in the Deployer code.

Is there any specific reason why the code doesn't use type declarations in many functions?

For instance, right now we have:

 *
 * @param string $command
 * @param array $options
 * @return string
 */
function run($command, $options = [])

From what I've seen in composer.json, the project requires PHP 7.3, so backwards compatibility with previous PHP versions, that may not support some declarations shouldn't be an issue.

Maybe the code could be improved by adding the type declarations in the functions, for instance:

function run(string $command, array $options = []): string

I understand that this will probably break things for some people, but given that Deployer is working towards version 7.0, it might be a good time for this change?

[antonmedv]

Yes, it’s good to change. Reason mostly is legacy code.

[bsacharski]

How would you prefer for this to be approached? One giant PR with all/most of the code refactored to include type declarations, or smaller ones that target one/few files?

[antonmedv]

Smaller one, as it easier to review. Plus kess merge conflicts.