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

Add missing status codes to logs #830

Merged
merged 6 commits into from Jun 13, 2018
Merged

Conversation

faustinoaq
Copy link
Contributor

Description of the Change

Add missing status codes to logs and some code cleanup

Alternate Designs

No

Benefits

Fixes #829

Possible Drawbacks

No

@faustinoaq faustinoaq requested a review from eliasjpr May 29, 2018 13:19
@faustinoaq faustinoaq changed the title Add missing status codes on logs Add missing status codes to logs May 29, 2018
@faustinoaq faustinoaq requested review from a team May 29, 2018 13:20
@faustinoaq faustinoaq added this to To Do in Framework 2018 via automation May 29, 2018
@faustinoaq faustinoaq moved this from To Do to In progress in Framework 2018 May 29, 2018
Copy link
Contributor

@eliasjpr eliasjpr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small comment

@@ -52,11 +52,12 @@ module Amber
private def http_status(status)
case status
when 200
text = "200 ".colorize(:green)
status.colorize(:green)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought I left a comment on this. Lets use range instead.

case status
when 200..299 then status.colorize(:green)
when 300..399 then status.colorize(:blue)
when 400..499 then status.colorize(:yellow)
when 500..599  status.colorize(:red)
else status.colorize(:white)
end

@robacarp robacarp merged commit 7aea00e into master Jun 13, 2018
Framework 2018 automation moved this from In progress to Done Jun 13, 2018
@robacarp robacarp deleted the fa/add-missing-status-logs branch June 13, 2018 16:35
@faustinoaq faustinoaq added this to the Version 0.8.0 milestone Jun 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants