Skip to content

Correct typespecs #177

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

Merged
merged 2 commits into from
Dec 8, 2021
Merged

Correct typespecs #177

merged 2 commits into from
Dec 8, 2021

Conversation

reachfh
Copy link
Contributor

@reachfh reachfh commented Dec 2, 2021

This corrects the typespecs, making it work with Dialyzer.

You can add Dialyzer to the project itself in dev and test environments (but not prod) by adding this to deps():

{:dialyxir, "~> 1.0", only: [:dev, :test], runtime: false}

then run

mix dialyzer

@@ -46,7 +46,7 @@ defmodule Exqlite.Sqlite3 do
case Sqlite3NIF.execute(conn, String.to_charlist(sql)) do
:ok -> :ok
{:error, reason} -> {:error, reason}
_ -> {:error, "unhandled error"}
# _ -> {:error, "unhandled error"}
Copy link
Member

Choose a reason for hiding this comment

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

Why was the fall through case commented out?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The only possible returns from execute are covered.

Copy link
Member

@warmwaffles warmwaffles left a comment

Choose a reason for hiding this comment

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

Looks good, just need two questions addressed

@warmwaffles warmwaffles added the enhancement New feature or request label Dec 2, 2021
@reachfh
Copy link
Contributor Author

reachfh commented Dec 8, 2021

Updated branch

@warmwaffles warmwaffles merged commit 5870b15 into elixir-sqlite:main Dec 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants