Skip to content

Refactor part 4 - #124

Merged
Thomasdezeeuw merged 20 commits into
rust-lang:masterfrom
Thomasdezeeuw:refactor-p4
Nov 11, 2020
Merged

Refactor part 4#124
Thomasdezeeuw merged 20 commits into
rust-lang:masterfrom
Thomasdezeeuw:refactor-p4

Conversation

@Thomasdezeeuw

@ThomasdezeeuwThomasdezeeuw commented Nov 9, 2020

Copy link
Copy Markdown
Collaborator

It's easiest to review by commit, those are small self-contained changes.

@Thomasdezeeuw

Thomasdezeeuw commented Nov 9, 2020

Copy link
Copy Markdown
CollaboratorAuthor

Main benefit is add Type::no_inherit on Windows which can remove an additional call to Socket::set_no_inherit.

@Thomasdezeeuw
Thomasdezeeuw marked this pull request as ready for review November 9, 2020 16:50
@de-vri-es

de-vri-es commented Nov 9, 2020

Copy link
Copy Markdown
Contributor

It would be nice to get a resolution on #117 before breaking it again :(

This was referenced Nov 9, 2020
Closed
Closed
de-vri-es
de-vri-es reviewed Nov 10, 2020
View reviewed changes
Comment threadCargo.toml Outdated
Comment threadCargo.toml
Comment threadsrc/sys/windows.rs
Comment threadsrc/sys/windows.rs Outdated
Comment threadsrc/sys/unix.rs Outdated
Comment threadsrc/sys/unix.rs Outdated
Comment threadsrc/socket.rs
@ThomasdezeeuwThomasdezeeuw mentioned this pull request Nov 11, 2020
Closed
@de-vri-esde-vri-es mentioned this pull request Nov 11, 2020
Closed
@Thomasdezeeuw
Change Socket::set_nosigpipe to accept a bool
adb1424
Same as Socket::set_nonblocking. Also add test for it.
@Thomasdezeeuw

Thomasdezeeuw commented Nov 11, 2020

Copy link
Copy Markdown
CollaboratorAuthor

@de-vri-es I've address your comments and added some tests (although more will be added in the future).

@de-vri-es

de-vri-es commented Nov 11, 2020

Copy link
Copy Markdown
Contributor

Looks good. I'd still go with exclude = ["tests"] instead of include = [$everything_else], but that's up to you :)

I'll start on rebasing #117 and implementing the requested changes.

@Thomasdezeeuw
Thomasdezeeuw merged commit df8c14d into rust-lang:masterNov 11, 2020
@Thomasdezeeuw
Thomasdezeeuw deleted the refactor-p4 branch November 11, 2020 12:49
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@de-vri-esde-vri-esde-vri-es approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Thomasdezeeuw@de-vri-es