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

update type spec for middleware #181

Conversation

BenMorganIO
Copy link
Contributor

@BenMorganIO BenMorganIO commented Feb 21, 2018

Offense:

lib/tesla/middleware/tuples.ex:40: The inferred return type of call/3 ({'error',_} | {'ok',_}) has nothing in common with #{'__client__':=fun(), '__module__':=atom(), '__struct__':='Elixir.Tesla.Env', 'body':=_, 'headers':=#{binary()=>binary()}, 'method':='delete' | 'get' | 'head' | 'options' | 'patch' | 'post' | 'put' | 'trace', 'opts':=[any()], 'query':=[{atom() | binary(),binary() | [{atom() | binary(),binary() | [{_,_}]}]}], 'status':=integer(), 'url':=binary()}, which is the expected return type for the callback of the 'Elixir.Tesla.Middleware' behaviour

The return types from Tesla.Middleware was only Tesla.Env.t. However, Tesla.Middleware.Tuples was returning {:ok, Env.t} | {:error, any()}. I've updated the return spec for this. While I was doing this, I updated some of the docs; line endings and s/transations/transactions.

@BenMorganIO BenMorganIO force-pushed the update-type-spec-for-middleware branch from 4c24b0b to 959c7e5 Compare February 21, 2018 20:34
@teamon
Copy link
Member

teamon commented Feb 21, 2018

Hi again :)

I appreciate the changes but please keep them to the minimum - I am working on a 1.0 branch and I'd like to have as few merge conflicts as possible.

The Tuples middleware is a special one and this is a known type error (see #109).
I'd rather keep it as a exception and not change the Tesla.Middleware spec.

@codecov
Copy link

codecov bot commented Feb 21, 2018

Codecov Report

Merging #181 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #181   +/-   ##
=======================================
  Coverage   98.33%   98.33%           
=======================================
  Files          21       21           
  Lines         421      421           
=======================================
  Hits          414      414           
  Misses          7        7
Impacted Files Coverage Δ
lib/tesla/middleware/tuples.ex 100% <ø> (ø) ⬆️
lib/tesla.ex 100% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0e2e1f4...959c7e5. Read the comment docs.

@teamon
Copy link
Member

teamon commented Mar 13, 2018

@BenMorganIO I'm closing this. Please take a look at 1.0 branch as it solves this issue.

@teamon teamon closed this Mar 13, 2018
@BenMorganIO BenMorganIO deleted the update-type-spec-for-middleware branch March 14, 2018 03:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants