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

Actually use 'reponse' in _conditional_error() method #76

merged 1 commit into from Mar 12, 2018


None yet
2 participants

jonathn commented Mar 9, 2018

  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
    Bug fix

  • What is the related issue number (starting with #)

  • What is the current behavior? (You can also link to an open issue here)
    Bug was introduced during this refactoring:

The response parameter is never used, instead the hardcoded string '408 Request Timeout' is always sent.

  • What is the new behavior (if this is a feature change)?

Sends 'response' instead

  • Other information:

This comment has been minimized.

codecov bot commented Mar 9, 2018

Codecov Report

Merging #76 into master will decrease coverage by 0.5%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master      #76      +/-   ##
- Coverage   65.67%   65.16%   -0.51%     
  Files          15       15              
  Lines        2753     2753              
- Hits         1808     1794      -14     
- Misses        945      959      +14

This comment has been minimized.


webknjaz commented Mar 12, 2018

Thanks! May I ask you to also add a test for it please?

@webknjaz webknjaz added the bug label Mar 12, 2018


This comment has been minimized.


webknjaz commented Mar 12, 2018

Nevermind, there's no easy way to add test now

@webknjaz webknjaz merged commit 405ccb5 into cherrypy:master Mar 12, 2018

8 checks passed

WIP ready for review
ci/circleci: linux-build Your tests passed on CircleCI!
ci/circleci: macos-build Your tests passed on CircleCI!
codeclimate All good!
codecov/patch 100% of diff hit (target 65.67%)
continuous-integration/appveyor/pr AppVeyor build succeeded
continuous-integration/travis-ci/pr The Travis CI build passed
lgtm analysis: Python No alert changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment