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

inet_address: add new_from_bytes constructor #201

Merged

Conversation

Projects
None yet
4 participants
@abdulrehman-git
Copy link
Contributor

commented Mar 9, 2019

No description provided.

@GuillaumeGomez

This comment has been minimized.

Copy link
Member

commented Mar 9, 2019

Looks good, thanks!

cc @sdroege @EPashkin

@EPashkin

This comment has been minimized.

Copy link
Member

commented Mar 10, 2019

@abdulrehman-git Thanks
👍

@@ -49,6 +49,7 @@ mod subprocess;
mod subprocess_launcher;
#[cfg(any(unix, feature = "dox"))]
mod unix_socket_address;
mod inet_address;

This comment has been minimized.

Copy link
@sdroege

sdroege Mar 10, 2019

Member

Don't we need a pub use inet_address::InetAddressOctet here then?

This comment has been minimized.

Copy link
@abdulrehman-git

abdulrehman-git Mar 10, 2019

Author Contributor

Ah, let me fix.

use InetAddress;

#[derive(Debug)]
pub enum InetAddressOctet<'a> {

This comment has been minimized.

Copy link
@sdroege

sdroege Mar 10, 2019

Member

The name does not seem ideal. It's not a single octet (== byte) but 4 or 16 bytes. Maybe instead of using the rare word octet, we could call this InetAddressBytes instead?

This comment has been minimized.

Copy link
@abdulrehman-git

abdulrehman-git Mar 10, 2019

Author Contributor

Okay, let me rename :-), I used Octet to match ip.octets().

This comment has been minimized.

Copy link
@sdroege

sdroege Mar 10, 2019

Member

@EPashkin @GuillaumeGomez do you have suggestions? I would call it bytes because that's how it's called in GIO :)

This comment has been minimized.

Copy link
@EPashkin

EPashkin Mar 10, 2019

Member

IMHO Octets or Bytes is almost same, Octet is really not.
https://en.wikipedia.org/wiki/IP_address uses "group of 8 bits (an octet)" but not byte maybe because byte can have different size.
So I for InetAddressOctets

This comment has been minimized.

Copy link
@GuillaumeGomez

GuillaumeGomez Mar 10, 2019

Member

In french, octet is the translation of byte so it seemed good to me. :3

For the name, InetAddressData might be enough?

This comment has been minimized.

Copy link
@sdroege

sdroege Mar 10, 2019

Member

@EPashkin That was true decades ago, nowadays a byte is always 8 bit fortunately. And the GIO API talks about bytes here (new_from_bytes() for example), not octets.

This comment has been minimized.

Copy link
@EPashkin

EPashkin Mar 10, 2019

Member

Ok, then Bytes, just Data IMHO not clear

This comment has been minimized.

Copy link
@abdulrehman-git

abdulrehman-git Mar 10, 2019

Author Contributor

I had force-pushed with InetAddressBytes :-).

@abdulrehman-git abdulrehman-git force-pushed the abdulrehman-git:inet-address-new-from-bytes-constructor branch from 758f6da to c4e04d5 Mar 10, 2019

@sdroege sdroege merged commit 24c71d4 into gtk-rs:master Mar 10, 2019

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@abdulrehman-git abdulrehman-git deleted the abdulrehman-git:inet-address-new-from-bytes-constructor branch Mar 10, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.