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

Introduce new 'halt_on_error' to overload exit #4383

Merged
merged 3 commits into from Oct 2, 2017

Conversation

2 participants
@danielbachhuber
Member

danielbachhuber commented Sep 29, 2017

Change optional exception handling from `throw_exception_on_error` to…
… `halt_on_error`.

Halting execution is the special case handling, not letting exceptions bubble up.

@schlessera schlessera changed the title from Introduce new 'throw_exception_on_error' to overload exit to Introduce new 'halt_on_error' to overload exit Sep 29, 2017

@danielbachhuber

This comment has been minimized.

Show comment
Hide comment
@danielbachhuber

danielbachhuber Oct 2, 2017

Member

@schlessera Are you planning to add tests for this too?

Member

danielbachhuber commented Oct 2, 2017

@schlessera Are you planning to add tests for this too?

@schlessera

This comment has been minimized.

Show comment
Hide comment
@schlessera

schlessera Oct 2, 2017

Member

@danielbachhuber Same issue for testing as here: wp-cli/core-command#38

I looked into dropping time-out for cURL to 1ms, but that doesn't work reliably, it often falls back to 1s on some systems.

So, right now, I'd say we avoid these tests to not cause unnecessary HTTP requests.

Member

schlessera commented Oct 2, 2017

@danielbachhuber Same issue for testing as here: wp-cli/core-command#38

I looked into dropping time-out for cURL to 1ms, but that doesn't work reliably, it often falls back to 1s on some systems.

So, right now, I'd say we avoid these tests to not cause unnecessary HTTP requests.

@schlessera

This comment has been minimized.

Show comment
Hide comment
@schlessera

schlessera Oct 2, 2017

Member

@danielbachhuber Feel free to merge if you agree, or let's discuss the merits of tests and their implementations otherwise.

Member

schlessera commented Oct 2, 2017

@danielbachhuber Feel free to merge if you agree, or let's discuss the merits of tests and their implementations otherwise.

@danielbachhuber danielbachhuber merged commit 7eb9834 into master Oct 2, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@danielbachhuber danielbachhuber deleted the 35-throw-exception branch Oct 2, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment