-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Use DisplayBuffer
for socket addresses.
#100640
Conversation
(rust-highfive has picked a reviewer for you, use r? to override) |
Hey! It looks like you've submitted a new PR for the library teams! If this PR contains changes to any Examples of
|
4af3809
to
6292967
Compare
r? @thomcc |
Can you add tests covering long addresses? E.g. ones that would catch an issue if the length used by the Also, to be clear (since it's not clear from the PR description), none of the renamed things are public API, this modifies internal stuff only. That said, the renames are probably not that well motivated (since |
I added some tests for short and long addresses.
I think I guess it would also make sense for |
99bf5e4
to
5070e6a
Compare
I'm not sure this level of nesting helps? |
How do you suggest to structure this? |
5070e6a
to
de12062
Compare
I think what you had initially was probably better, but honestly I think just the changes so that DisplayBuffer is used for the parsing would be fine. |
☔ The latest upstream changes (presumably #100881) made this pull request unmergeable. Please resolve the merge conflicts. |
de12062
to
42a482a
Compare
This comment has been minimized.
This comment has been minimized.
42a482a
to
d61ecec
Compare
I flattened the module structure again, but did rename |
Thank you. This looks great. Will approve after CI goes green. |
@bors r+ |
3253d26
to
24e59f6
Compare
24e59f6
to
14230a7
Compare
@rustbot ready |
@Jarcho is this what you were looking for? |
Clippy changes look good. |
@bors r+ |
☀️ Test successful - checks-actions |
Finished benchmarking commit (c815756): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Footnotes |
…r=thomcc Use `DisplayBuffer` for socket addresses. Continuation of rust-lang#100625 for socket addresses. Renames `net::addr` to `net::addr::socket`, `net::ip` to `net::addr::ip` and `net::ip::display_buffer::IpDisplayBuffer` to `net::addr::display_buffer::DisplayBuffer`.
Continuation of #100625 for socket addresses.
Renames
net::addr
tonet::addr::socket
,net::ip
tonet::addr::ip
andnet::ip::display_buffer::IpDisplayBuffer
tonet::addr::display_buffer::DisplayBuffer
.