Skip to content

More Dialyzer#132

Merged
jlouis merged 4 commits intoEonblast:masterfrom
jkingsbery:master
Mar 24, 2014
Merged

More Dialyzer#132
jlouis merged 4 commits intoEonblast:masterfrom
jkingsbery:master

Conversation

@jkingsbery
Copy link
Copy Markdown

  1. Added types for (most of) the greeting record.
  2. In dj_al's commit, there was a check the encoding, but running dialyzer shows that the encoding can only ever be an atom, so the other checks in the pattern match were superfluous. I made the simplest change to get dialyzer passing again, but now the encoding_ok check seems superfluous (again, because the encoding field has to be an atom for dialyzer to pass). Should make dialyzer be added to the travis build?

@jkingsbery
Copy link
Copy Markdown
Author

Hmm... I'll take a look at this tomorrow to see why it's failing.

@jlouis
Copy link
Copy Markdown
Collaborator

jlouis commented Mar 20, 2014

Ok. I think we should consider adding dialyzer to the travis build if possible. Since we are dialyzer-clean, we better keep it so. @dj_al, did you try running the dialyzer on this?

@jkingsbery
Copy link
Copy Markdown
Author

The test that @dj_al added has a value for encoding that doesn't match the dialyzer types. We either need to change the test to match the types or fix the types in emysql.hrl to match the allowed encodings.

@jlouis
Copy link
Copy Markdown
Collaborator

jlouis commented Mar 22, 2014

Hmm, what do you propose we do here? I am all for making these things align up.

@jkingsbery
Copy link
Copy Markdown
Author

I copied the type info over from the function spec. Dialyzer and tests both pass now.

@jlouis
Copy link
Copy Markdown
Collaborator

jlouis commented Mar 24, 2014

Awesome, pulling!

jlouis added a commit that referenced this pull request Mar 24, 2014
@jlouis jlouis merged commit 23df444 into Eonblast:master Mar 24, 2014
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