Skip to content

Conversation

@weitzman
Copy link
Member

I think its OK to remove these during a minor release since they are long ancient and non-recommended APIs. Happy to discuss.

One notable change is that pm:security now exits with 0 when there an update is found. This fixes #3749. I had to do this since I can no longer set a 1 exit code AND print my structured output, even when the command returns a ResultData object.

@greg-1-anderson
Copy link
Member

Are we just giving up on the "Drush command terminated abnormally due to an unrecoverable error" feature? While that caused a lot of confusion, not having any indication that the website called exit() might be even worse.

I'm not sure about changing the exit-status behavior of pm:security in a minor release, though. This could interfere with folks' security-update checking scripts. It might be better to enhance annotated command / output formatters to allow for returning structured data with an exit code.

@greg-1-anderson
Copy link
Member

Here's a PR in annotated command that would allow both an exit code and result data to be returned by the same annotated command:

consolidation/annotated-command#167

If this works well for our purpose, then I could make a stable release with this addition.

@weitzman
Copy link
Member Author

That looks great. Thanks Greg.

@greg-1-anderson
Copy link
Member

I'm okay with losing "Drush command terminated abnormally due to an unrecoverable error" if you really want to, but I sort of think we should keep it. We could try without for a while and see how it goes.

@weitzman
Copy link
Member Author

I brought it back in recent commits. https://github.com/drush-ops/drush/pull/3780/files#diff-00e1d309f15af5b596b61de17b456130R36

@weitzman weitzman merged commit 85826fa into master Nov 15, 2018
@weitzman weitzman deleted the exitCodeCompleted branch November 15, 2018 02:30
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.

drush pm:security -n wrong status-code

3 participants