Skip to content
This repository has been archived by the owner on May 9, 2023. It is now read-only.

Full test coverage multiaddr and fixes #9

Merged
merged 6 commits into from
May 4, 2016
Merged

Full test coverage multiaddr and fixes #9

merged 6 commits into from
May 4, 2016

Conversation

fredthomsen
Copy link
Contributor

I have gotten an interest in ipfs and enhancing python and landed here.

Added some unit tests and found an issue in multiaddr.decapsulate throwing a ValueError.

@codecov-io
Copy link

codecov-io commented May 1, 2016

Current coverage is 90.34%

Merging #9 into master will increase coverage by +3.65%

  1. File ...ltiaddr/multiaddr.py was modified. more
    • Misses -7
    • Partials -1
    • Hits +6
@@             master         #9   diff @@
==========================================
  Files             5          5          
  Lines           353        352     -1   
  Methods           0          0          
  Messages          0          0          
  Branches         63         61     -2   
==========================================
+ Hits            306        318    +12   
+ Misses           30         21     -9   
+ Partials         17         13     -4   

Powered by Codecov. Last updated by 4f08797...b43507d

@fredthomsen
Copy link
Contributor Author

I don't understand why this codecov is complaining about this change...

self._bytes = bytes_addr
else:
raise ValueError("Invalid address type, must be bytes or str")
raise ValueError("Invalid address type, must be bytes xor str")
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't like this wording change, it reads like the user is supposed to pass in b'foo' ^ "bar". I think most people parse or as an exclusive or, anyway.

Really I ought to fix this constructor to only take a single parameter, but the way python handles bytes and strings makes it basically impossible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Understood. I struggled a bit with that. Yes, it would be much better if only one parameter was taken as an input, but like you said not sure of a good way to do that works in both python 2 and 3.

@sbuss
Copy link
Collaborator

sbuss commented May 2, 2016

Thanks for the PR! Patches that add a bunch of tests are my favorite :)

I'm looking in to the codecov failure. I think I misconfigured it when I set it up.

Please address my comments and rebase against master. I think master should fix the codecov issue.

@fredthomsen
Copy link
Contributor Author

Updates made and rebase done.

@sbuss
Copy link
Collaborator

sbuss commented May 4, 2016

LGTM, thanks for the patch!

I'm going to ignore the codecov failure. I'm still not sure why it's happening.

I'll add you to AUTHORS and prepare a new release today.

@sbuss sbuss merged commit 818db9f into multiformats:master May 4, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants