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 dialyxir and fix all the typespecs #46

Merged
merged 1 commit into from
Dec 18, 2017

Conversation

doughsay
Copy link
Contributor

@doughsay doughsay commented Dec 16, 2017

I was trying to use dialyzer in a project of mine using this library and noticed errors that I couldn't fix without touching this library. Here are some fixes I think will help, but there's plenty of room for other improvements. At least I think this is a good start.

The main issues:

  • Map.t doesn't exist, it should just be map
  • [] I think matches only "empty list". [any] is a generic "list of zero or more anything"
  • The exception stuff was a bit squirrelly, I opted to just treat it like a behavior implementation
  • Because new/1 was being defined in Braintree.Construction, I removed the typespecs for all the overridden ones, because dialyzer can't handle more than one typespec per function

Hope this helps, let me know if you'd like anything changed! ;)

Copy link
Owner

@sorentwo sorentwo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome! Thank you for taking the time, and having the diligence, to see all of these fixes through. The only bit I'd rather not have in here is the version bump, but as it isn't being released yet anyhow I don't see any harm.

@sorentwo sorentwo merged commit a09150f into sorentwo:master Dec 18, 2017
@doughsay
Copy link
Contributor Author

Sorry! My mistake, the version bump was not meant to be there. Thanks for merging though!

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