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.

Sign up for GitHub

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

Jump to bottom

This should address issue 112 #178

Closed
wants to merge 2 commits into from
Closed

This should address issue 112 #178

wants to merge 2 commits into from

Conversation

hiromipaw
Copy link
Contributor

After a quick chat on Friday with @ixti, thought to address issue 112: #112
Please let me know if I haven't understood the specs correctly and if anything is missing.

@sferik
Copy link
Contributor

sferik commented Mar 2, 2015

Overall, this looks good. However, it pushes the HTTP::Redirector#follow method over our complexity metric, which causes the build to fail. Can you please refactor this so RuboCop doesn’t complain? To check the complexity locally, you can run the following command from the http.rb project directory.

bundle exec rubocop lib/http/redirector.rb

@hiromipaw
Copy link
Contributor Author

Sure I will refactor and push back.

@ixti
Copy link
Member

ixti commented Mar 2, 2015

@hiromipaw Thank you very much. I'll review PR later this week.

@ixti ixti mentioned this pull request Mar 16, 2015
Merged
@ixti
Copy link
Member

ixti commented Mar 16, 2015

Thanks again. I have merged your commits into my branch:
https://github.com/httprb/http.rb/tree/improvement/redirector-logic

Gonna improve couple of more things and will merge it to master.
Thanks again!

@ixti ixti closed this Mar 16, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@hiromipaw @sferik @ixti