Skip to content

Conversation

@ColonelThirtyTwo
Copy link
Contributor

Breaking change.

  • Implement a new module connection, which abstracts std::net types into ones that can use
    std::os::unix::net types on Unix platforms.
  • Change ServerConfig::addr to a new, abstracted type and add http_unix method.
    http and https methods are unchanged and should still work.
  • Request::remote_addr now returns Option<&SocketAddr>. This is Some for TCP servers and
    None for UNIX servers (since UNIX remote sockets are almost always unnamed).

TODO: test on Windows to make sure everything still works. Will do that this evening.

Breaking change.

* Implement a new module `connection`, which abstracts `std::net` types into ones that can use
  `std::os::unix::net` types on Unix platforms.
* Change `ServerConfig::addr` to a new, abstracted type and add `http_unix` method.
  `http` and `https` methods are unchanged and should still work.
* `Request::remote_addr` now returns `Option<&SocketAddr>`. This is `Some` for TCP servers and
  `None` for UNIX servers (since UNIX remote sockets are almost always unnamed).
Copy link
Member

@bradfier bradfier left a comment

Choose a reason for hiding this comment

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

LGTM, one comment about our excessive use of inline in this code but you're really following the convention so it's not something you need to change.

I'll merge this and work on a short migration guide for users of 0.11

}

#[cfg(unix)]
#[inline]
Copy link
Member

Choose a reason for hiding this comment

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

I know there are #[inline] scattered all over this project but they're really unnecessary, I should do a cleanup where I remove them...

@bradfier
Copy link
Member

bradfier commented Apr 2, 2022

As you note actually, we need to check this works on Windows before we do a release, I'll try and get to that this weekend.

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.

2 participants