Skip to content

Conversation

@mickel8
Copy link
Contributor

@mickel8 mickel8 commented Mar 11, 2024

No description provided.

@codecov
Copy link

codecov bot commented Mar 11, 2024

Codecov Report

Merging #28 (217e24b) into master (3d98186) will decrease coverage by 1.36%.
The diff coverage is 74.54%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #28      +/-   ##
==========================================
- Coverage   79.59%   78.24%   -1.36%     
==========================================
  Files          17       18       +1     
  Lines         892     1002     +110     
==========================================
+ Hits          710      784      +74     
- Misses        182      218      +36     
Files Coverage Δ
lib/ex_ice/dns/message.ex 74.54% <74.54%> (ø)

... and 3 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3d98186...217e24b. Read the comment docs.

@mickel8 mickel8 force-pushed the dns branch 2 times, most recently from 54eda54 to 93114e6 Compare March 11, 2024 17:22
@mickel8 mickel8 marked this pull request as ready for review March 11, 2024 17:24
@mickel8 mickel8 requested a review from LVala March 11, 2024 17:24
@mickel8 mickel8 mentioned this pull request Mar 11, 2024
63 tasks
Comment on lines +156 to +158
defp decode_type(<<1::16, data::binary>>), do: {:ok, :a, data}
defp decode_type(<<2::16, data::binary>>), do: {:ok, :ns, data}
defp decode_type(<<3::16, data::binary>>), do: {:ok, :md, data}
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: wouldn't it be nicer to do @spec do_decode_type(integet()) :: {:ok, atom()} | :error like defp do_decode_type(1), do: {:ok, :a} or even validate the integer and just do defp do_decode_type(10), do: :a (do_descode_type is just and example name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On the one hand yes, but on the other hand I would have to handle this {:ok, class} | :error in decode_type so I am not sure that's worthy

@mickel8 mickel8 merged commit 60a28e7 into master Mar 12, 2024
@mickel8 mickel8 deleted the dns branch March 12, 2024 16:56
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.

3 participants