Pass options to dynamic block headers #661
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
TheSmartnik
reviewed
Jul 15, 2019
lib/httparty.rb
Outdated
| end | ||
| end | ||
|
|
||
| def process_dynamic_headers(headers) | ||
| headers.each_with_object({}) do |header, processed_headers| | ||
| def process_dynamic_headers(options) |
There was a problem hiding this comment.
Maybe it would be better to move it to a separate class? Something like HeaderProcessor. I don't really like that we do everything in a base class. Additionally, the method became more complex, so it's justified
There was a problem hiding this comment.
@TheSmartnik please preview code with extracted HeadersProcessor class
wojw5
force-pushed
the
pass-options-to-block-headers
branch
3 times, most recently
from
508f504
to
b9df779
Compare
wojw5
force-pushed
the
pass-options-to-block-headers
branch
from
b9df779
to
1d8d0a7
Compare
TheSmartnik
reviewed
Aug 30, 2019
| private | ||
|
|
||
| def process_dynamic_headers | ||
| options[:headers].each_with_object({}) do |header, processed_headers| |
There was a problem hiding this comment.
Just, fyi. You can do |(key, value), processed_headers|
|
@wojw5 thanks! I like what you did here, looks nice |
|
@TheSmartnik Do you think this is enough to bump version and do gem release? I have another gem that depends on this feature and i will release it to RubyGems once this dependency will be on RubyGems as well. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I encountered the following problem: API required checksum based on the request body.
This simple change allows using request options in block header like in this example: