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

Fix dialyzer warnings #6

Merged
merged 5 commits into from Mar 3, 2020
Merged

Conversation

@rustra
Copy link
Contributor

rustra commented Mar 2, 2020

PR adds missing type specs and ignores some Dialyzer warnings.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Mar 2, 2020

Coverage Status

Coverage increased (+0.007%) to 70.924% when pulling 08b4a44 on rustra:fix_dialyzer_warnings into b470172 on marcelotto:master.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Mar 2, 2020

Coverage Status

Coverage increased (+0.02%) to 70.942% when pulling 36df601 on rustra:fix_dialyzer_warnings into b470172 on marcelotto:master.

@rustra rustra mentioned this pull request Mar 2, 2020
@rustra rustra force-pushed the rustra:fix_dialyzer_warnings branch from 36df601 to 8345e10 Mar 2, 2020
Copy link
Owner

marcelotto left a comment

❤️ Great job.

I'm not that much experienced with dialyzer and since I wasn't sure about the usage of _t pre- and suffixes in type names. So I looked at some other projects (in particular Ecto) what's the practice there and they don't seem to have those. I also would prefer them without and some cases a bit more specific.

lib/rdf/blank_node/increment.ex Outdated Show resolved Hide resolved
lib/rdf/dataset.ex Outdated Show resolved Hide resolved
lib/rdf/dataset.ex Outdated Show resolved Hide resolved
lib/rdf/iri.ex Outdated Show resolved Hide resolved
lib/rdf/iri.ex Outdated Show resolved Hide resolved
lib/rdf/literal.ex Outdated Show resolved Hide resolved
lib/rdf/prefix_map.ex Outdated Show resolved Hide resolved
lib/rdf/graph.ex Outdated Show resolved Hide resolved
lib/rdf/graph.ex Outdated Show resolved Hide resolved
lib/rdf/dataset.ex Outdated Show resolved Hide resolved
@rustra rustra force-pushed the rustra:fix_dialyzer_warnings branch from 071080a to df30796 Mar 2, 2020
lib/rdf/prefix_map.ex Outdated Show resolved Hide resolved
@@ -35,11 +35,13 @@ defmodule RDF.Numeric do
@doc """
The list of all numeric datatypes.
"""
@dialyzer {:nowarn_function, types: 0}

This comment has been minimized.

Copy link
@marcelotto

marcelotto Mar 3, 2020

Owner

What about this? Doesn't it work with the suggested spec? I could live without it but I'm curious.

Copy link
Owner

marcelotto left a comment

We're almost there.

@rustra rustra force-pushed the rustra:fix_dialyzer_warnings branch from df30796 to 3da72f9 Mar 3, 2020
@rustra

This comment has been minimized.

Copy link
Contributor Author

rustra commented Mar 3, 2020

According this: #6 (comment)
Actually the problem is that I don't really understand how to deal with MapSet as it has @type t and @opaque t(value).
Dialyzer says: The call 'Elixir.MapSet':to_list(#{'__struct__' => 'Elixir.MapSet', map => #{...}) does not have an opaque term of type 'Elixir.MapSet':t(_) as 1st argument.
I believe that some beautiful solution should exist and somebody else will fix it later.

Copy link
Owner

marcelotto left a comment

Perfect. Thank you very much. 🚀

@marcelotto marcelotto merged commit a1071b9 into marcelotto:master Mar 3, 2020
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.007%) to 70.924%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.