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 bang methods #83

Merged
merged 5 commits into from
May 6, 2016
Merged

Conversation

bradleyd
Copy link
Contributor

@bradleyd bradleyd commented May 5, 2016

  • Add bang functions for HTTP requests.
  • Return ErrorResponse struct by default when there is an issue.
  • Add catch all for Response#success?

I bumped version to 2.3.0 not sure how you would want to handle it.

Bradleyd Smith added 3 commits May 4, 2016 09:07
…ct by default when there is an issue. add catch all for success? when params are not expected
@bradleyd
Copy link
Contributor Author

bradleyd commented May 5, 2016

now that I think about it..maybe the version should be bumped to 3.0.0 since technically they are breaking changes.

@valpackett
Copy link
Owner

About the version – I'll bump it to 3.0.0 later, please don't touch the version in pull requests.

Seems reasonable overall, except you're duplicating the raise code everywhere. Make a request! method that does the exception throwing and make other bang methods call it!

@bradleyd
Copy link
Contributor Author

bradleyd commented May 5, 2016

i will make changes and return the version back to what it was...sorry about that.

@bradleyd
Copy link
Contributor Author

bradleyd commented May 6, 2016

@myfreeweb better? BTW, thanks for the feedback.

@valpackett
Copy link
Owner

Uh… almost. Except not really better. Please don't add any code duplication — Don't Repeat Yourself! (or others)

I meant something like this:

      @doc """
      Like `request`, but raises  `HTTPotion.HTTPError` if failed.
      """
      @spec request!(atom, String.t, [{atom(), any()}]) :: %HTTPotion.Response{} | %HTTPotion.AsyncResponse{}
      def request!(method, url, options \\ []) do
        case request(method, url, options) do
          %HTTPotion.ErrorResponse{message: message} ->
            raise HTTPotion.HTTPError, message: message
          response -> response
        end
      end

Also, the @spec for the non-bang request should include %HTTPotion.ErrorResponse{} in the list of return types.

@bradleyd
Copy link
Contributor Author

bradleyd commented May 6, 2016

@myfreeweb 3rd time is a charm =]

@valpackett valpackett merged commit c2ed54f into valpackett:master May 6, 2016
@valpackett
Copy link
Owner

Looks good, thanks!

@bradleyd
Copy link
Contributor Author

bradleyd commented May 6, 2016

Thank you!

@chibicode
Copy link

@myfreeweb could you bump the version?

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.

None yet

3 participants