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 spec of write/3 of Instream.Connection.{QueryRunnerV1, QueryRunnnerV2} #74

Merged

Conversation

nallwhy
Copy link
Contributor

@nallwhy nallwhy commented Jan 10, 2022

Following Instream.Connection, write/3 of Instream.Connection.{QueryRunnerV1, QueryRunnnerV2} take its 3rd param as module not map.

      @impl Connection
      def write(points, opts \\ [])

      def write(point, opts) when is_map(point), do: write([point], opts)

      def write(points, opts) when is_list(points) do
        case config(:version) do
          :v2 -> QueryRunnerV2.write(points, opts, __MODULE__)
          _ -> QueryRunnerV1.write(points, opts, __MODULE__)
        end
      end

Those errors break my dialyzer.

lib/instream/connection.ex:109:call
The function call will not succeed.

Instream.Connection.QueryRunnerV2.write(_ :: any(), _ :: any(), MyModule)

breaks the contract
(map() | [map()], Keyword.t(), map()) :: any()

________________________________________________________________________________
lib/instream/connection.ex:110:call
The function call will not succeed.

Instream.Connection.QueryRunnerV1.write(_ :: any(), _ :: any(), MyModule)

breaks the contract
(map() | [map()], Keyword.t(), map()) :: any()

@coveralls
Copy link

coveralls commented Jan 10, 2022

Coverage Status

Coverage remained the same at 76.852% when pulling cefbac1 on nallwhy:fix_spec_of_write_of_query_runners into 05360f9 on mneudert:master.

@mneudert mneudert merged commit 2931bdf into mneudert:master Jan 10, 2022
@mneudert
Copy link
Owner

❤️ 💚 💙 💛 💜

At least the dialyzer output was rather helpful in this case. I will additionally try to modify the CI stage to check those cases with real modules. Was broken far too long to not catch it 😅

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.

None yet

3 participants