Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added file (--file and -f) option to deployer allowing to specify which deploy script to use #86

Closed
wants to merge 1 commit into from

Conversation

AlexStansfield
Copy link
Contributor

Issue #67

Will accept relative or absolute paths.

Example usage:
php bin/deploy --file=/home/alex/sites/deploy-scripts/wordpress.php deploy
php bin/deploy --file=deploy-scripts/anothersite.php deploy
php bin/deploy -fdeploy-scripts/wordpress.php deploy

@AlexStansfield
Copy link
Contributor Author

Hold off on the merge, I found a bug.

@AlexStansfield
Copy link
Contributor Author

from http://php.net/manual/en/function.realpath.php

realpath() returns FALSE on failure, e.g. if the file does not exist.

So if we've already determined the realpath call has failed it saves us from doing any other checks

@aohorodnyk
Copy link

both variants:

(is_file($file) && is_readable($file))

and

$file !== false && is_file($file) && is_readable($file)

have same behaviors, is_file and is_readable returns false if have parameter false
Your condition $var !== false is superfluous.

If you have another opinion, say please

@AlexStansfield
Copy link
Contributor Author

Check this line:

$deployFile = realpath($optionFile);

I'm using realpath in order to create an absolute path to the file no matter if you give it a relative path (--file=scripts/site.php) or an absolute one (--file=/var/deploy/site.php).

Realpath will return false if the file does not exist (or if the function fails for any other reason).

Therefore by doing a simple check to see if $deployFile is already false I save from having to hit the filesystem again by calling is_file.

If $realpath is false already then calling is_file is superfluous and just leads to a wasted filesystem call.

I guess it's a matter of which is more wasteful, a call to the file system or a boolean comparison.

@aohorodnyk
Copy link

Thanks @AlexStansfield, I understood your opinion, it's a small optimization.
But I think will be better don't use small optimizations and have more cleared code.
Thanks, Anton

@AlexStansfield
Copy link
Contributor Author

ok, I will remove it

@AlexStansfield
Copy link
Contributor Author

Due to the way that Symfony\Component\Console\Input\ArgvInput works this is actually a lot harder to implement than expected.

I'm not sure if there is a nice way without creating our own Input class.

The reason being that it's pretty strict about the options and arguments passed to it. However deploy.php file itself adds options and arguments. So if I try to use the name of the stage I want to deploy to the script fails on too many arguments as it's not imported the deploy.php file yet so doesn't the stage argument hasn't been defined.

@antonmedv
Copy link
Member

I'll will take your code to v3, and try to implement this without creation own Input class. Try to keep KISS. 👾

@AlexStansfield
Copy link
Contributor Author

Good luck

@antonmedv
Copy link
Member

Added in 9c37344

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants