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 network handling. #118

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix network handling. #118

wants to merge 1 commit into from

Conversation

Barre
Copy link

@Barre Barre commented Jan 11, 2020

Fixes:

  • is_subnet_of was broken if the supernet was not the network address.
  • Display now prints the network ip and not the ip used to construct the IpNetwork.
  • Tests that were asserting against .ip().

Adds:

  • A regression test.

If there is no plans on breaking backwards compatibility, the ip() method should be re-introduced in my changes. Also, if ip is not kept, there would be no need for network() to compute the network address anymore as the arithmetic can be done in the constructor and the resulting ip stored in the struct, discarding the original IpAddr.

Fixes:

- `is_subnet_of` was broken if the supernet was not the network address.
- Display now prints the network ip and not the initial ip.
- Tests that were asserting against `.ip()`.

Adds:

- A regression test.
@achanda
Copy link
Owner

achanda commented Jan 18, 2020

Sorry, it took me a while to reply, and thanks for the PR. I still have to figure out a good workflow of deprecating APIs and removing them eventually. Hence I am not a fan of changing the behaviour of ip right now.

@Barre
Copy link
Author

Barre commented Jan 18, 2020

@achanda I understand!

I'll update my PR with just the fix for is_subnet_of in the next few days then.

@ctrlcctrlv ctrlcctrlv mentioned this pull request Jun 13, 2023
@ctrlcctrlv
Copy link
Contributor

Probably superseded by #177.

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