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

Failing calls to the Shell now return a proper exit code #224

Merged
merged 1 commit into from May 27, 2016

Conversation

HavokInspiration
Copy link
Member

Refs #215

@thinkingmedia
Copy link

thinkingmedia commented May 20, 2016

Have you tested this? I've found that cake ignores the return value for main in a shell. I had to use exit($value) to get my shells to return a value.

@HavokInspiration
Copy link
Member Author

HavokInspiration commented May 20, 2016

It does. See this.
The key is to return a bool (true if success, false if error) because of how the ShellDispatcher treats the response of the inner method call.

You can clone the branch, run a shell without error and run echo $? it should return 0.
Create an error (remove an entry in the phinxlog for instance and re-run a migrate call) and run echo $? it should return 1.

And given than shell can be dispatched from other shell, using exit is not an option. Whether or not to early exit the entire shell call should come from the user and not the plugin itself.

@thinkingmedia
Copy link

thinkingmedia commented May 20, 2016

Ah, okay. I was trying to have the shell return an exit code as a value (i.e. how many records exist). Which doesn't work because ShellDispatcher mutates the return value to 0 or 1.

I don't think that will effect your changes.

@HavokInspiration
Copy link
Member Author

I don't think that will effect your changes.

What do you mean ?

@HavokInspiration HavokInspiration modified the milestones: 1.6.1, 1.6.2 May 22, 2016
@HavokInspiration
Copy link
Member Author

Anyone to give this a quick review ?

@dereuromark
Copy link
Member

Looks good to me.

@HavokInspiration HavokInspiration merged commit f219691 into master May 27, 2016
@HavokInspiration HavokInspiration deleted the issue-215 branch May 27, 2016 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants