Skip to content

Fixed typespecs, bumped HTTPoison to 1.0 so the header signature matches what is used - #1

Open
Narnach wants to merge 3 commits into
thomasdola:masterfrom
betacooperation:master
Open

Fixed typespecs, bumped HTTPoison to 1.0 so the header signature matches what is used#1
Narnach wants to merge 3 commits into
thomasdola:masterfrom
betacooperation:master

Conversation

@Narnach

@Narnach Narnach commented Feb 8, 2018

Copy link
Copy Markdown

When running Dialyzer against code that used Birdy, we got some strange errors resulting from Birdy's typespecs being inconsistent with the actual behaviour of the code, and its interactions with other libraries. This commit contains the changes we made in order to make Dialyzer happy with Birdy.

HTTPoison version < 1.0 only allowed binary header data, such as a %{"binary" => "binary"} or [{"binary", "binary"}]. In 1.0 it is also possible to pass in atoms, such as used in Birdy. HTTPoison as dependency was bumped to make this work as expected.

Typespecs in Birdy had two main issues:

  • some were overly broad, so atom was used where a specific :ok or :form or :error would suffice. These have been made more specific, so Dialyzer can help to check that case statements matching the Birdy responses are handled correctly.
  • some were factually incorrect when compared to some functions’ return types, especially when interacting with other libraries

mix dialyzer is now happy and purring like a kitten, and the tests are still passing as well.

marceldegraaf and others added 3 commits October 18, 2017 10:51
…hes what is used

HTTPoison < 1.0 only allowed binary header data, such as a `map(binary => binary)` or `[{binary, binary}]`. In 1.0 it is also possible to pass in atoms, such as used in Birdy.

Typespecs in Birdy had two main issues:

* they were overly broad, so `atom` was used where a specific `:ok` or `:form` or `:error` would suffice
* they were factually incorrect when compared to some functions’ return types

`mix dialyzer` is now happy and purring like a kitten.
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.

2 participants