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

Respect default_options for HTTP::Chainable#request and HTTP::Chainable#build_request #515

Merged
merged 2 commits into from
Dec 29, 2018

Conversation

RickCSong
Copy link
Contributor

@RickCSong RickCSong commented Dec 27, 2018

Currently, if default_options are set, they are respected for methods such as #via or #timeout but not with a simple request.

Let me know if this would make sense, or if the current implementation is intentional!

cc @zanker HI ZANKER :)

@RickCSong
Copy link
Contributor Author

RickCSong commented Dec 27, 2018

Removed some options from .travis.yml to fix builds

See error log: https://travis-ci.org/httprb/http/jobs/472558089

.travis.yml Show resolved Hide resolved
end

# Prepare an HTTP request with the given verb
# @param verb
# @param uri
# @option options [Hash]
def build_request(verb, uri, options = {}) # rubocop:disable Style/OptionHash
branch(options).build_request verb, uri
branch(default_options.merge(options)).build_request verb, uri
Copy link
Member

Choose a reason for hiding this comment

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

In both cases this and above, it will be more effiecient to pass options to delegated method instead:

# @param (see Client#build_request)
def build_request(*args)
  branch(default_options).build_request(*args)
end

This way you will avoid double-merging with no good. :D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call!! Updated.

Copy link
Member

@ixti ixti left a comment

Choose a reason for hiding this comment

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

Please, address given comment about default options and we're good here. :D

@zanker
Copy link
Contributor

zanker commented Dec 27, 2018

👋

`--no-rdoc` and `--no-ri` no longer work.
Copy link
Member

@ixti ixti left a comment

Choose a reason for hiding this comment

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

LGTM.

@ixti ixti merged commit 5187a1d into httprb:master Dec 29, 2018
@janko
Copy link
Member

janko commented Jan 9, 2019

It seems that there are tests missing for this change?

@ixti
Copy link
Member

ixti commented Jan 9, 2019

@janko-m good catch. we definitely need to do that.

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Jun 7, 2020
Update ruby-http to 4.4.1.


## 4.4.1 (2020-03-29)

* Backport [#590](httprb/http#590)
  Fix parser failing on some edge cases.
  ([@ixti])

## 4.4.0 (2020-03-25)

* Backport [#587](httprb/http#587)
  Fix redirections when server responds with multiple Location headers.
  ([@ixti])

* Backport [#599](httprb/http#599)
  Allow passing HTTP::FormData::{Multipart,UrlEncoded} object directly.
  ([@ixti])

## 4.3.0 (2020-01-09)

* Backport [#581](httprb/http#581)
  Add Ruby-2.7 compatibility.
  ([@ixti], [@janko])


## 4.2.0 (2019-10-22)

* Backport [#489](httprb/http#489)
  Fix HTTP parser.
  ([@ixti], [@fxposter])


## 4.1.1 (2019-03-12)

* Add `HTTP::Headers::ACCEPT_ENCODING` constant.
  ([@ixti])


## 4.1.0 (2019-03-11)

* [#533](httprb/http#533)
  Add URI normalizer feature that allows to swap default URI normalizer.
  ([@mamoonraja])


## 4.0.5 (2019-02-15)

* Backport [#532](httprb/http#532) from master.
  Fix pipes support in request bodies.
  ([@ixti])


## 4.0.4 (2019-02-12)

* Backport [#506](httprb/http#506) from master.
  Skip auto-deflate when there is no body.
  ([@Bonias])


## 4.0.3 (2019-01-18)

* Fix missing URL in response wrapped by auto inflate.
  ([@ixti])

* Provide `HTTP::Request#inspect` method for debugging purposes.
  ([@ixti])


## 4.0.2 (2019-01-15)

* [#506](httprb/http#506)
  Fix instrumentation feature.
  ([@paul])


## 4.0.1 (2019-01-14)

* [#515](httprb/http#515)
  Fix `#build_request` and `#request` to respect default options.
  ([@RickCSong])


## 4.0.0 (2018-10-15)

* [#482](httprb/http#482)
  [#499](httprb/http#499)
  Introduce new features injection API with 2 new feaures: instrumentation
  (compatible with ActiveSupport::Notification) and logging.
  ([@paul])

* [#473](httprb/http#473)
  Handle early responses.
  ([@janko-m])

* [#468](httprb/http#468)
  Rewind `HTTP::Request::Body#source` once `#each` is complete.
  ([@ixti])

* [#467](httprb/http#467)
  Drop Ruby 2.2 support.
  ([@ixti])

* [#436](httprb/http#436)
  Raise ConnectionError when writing to socket fails.
  ([@janko-m])

* [#438](httprb/http#438)
  Expose `HTTP::Request::Body#source`.
  ([@janko-m])

* [#446](httprb/http#446)
  Simplify setting a timeout.
  ([@mikegee])

* [#451](httprb/http#451)
  Reduce memory usage when reading response body.
  ([@janko-m])

* [#458](httprb/http#458)
  Extract HTTP::Client#build_request method.
  ([@tycoon])

* [#462](httprb/http#462)
  Fix HTTP::Request#headline to allow two leading slashes in path.
  ([@scarfacedeb])

* [#454](httprb/http#454)
  [#464](httprb/http#464)
  [#384](httprb/http#384)
  Fix #readpartial not respecting max length argument.
  ([@janko-m], [@marshall-lee])
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.

4 participants