-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Add doc for impl From for Addr #53522
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
src/libstd/net/addr.rs
Outdated
/// Converts a [`SocketAddrV4`] into a [`V4 variant of SocketAddr`]. | ||
/// | ||
/// [`SocketAddrV4`]: struct.SocketAddrV4.html | ||
/// [`V4 variant of SocketAddr`]: enum.SocketAddr.html#variant.V4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for my noob question as I haven't touched rust for quite a while.
Is this syntax valid and necessary for the docs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on running cargo doc
on the following, yes I believe it is valid as written.
pub enum SocketAddr {
V4,
}
/// Converts a `SocketAddrV4` into a [`V4 variant of SocketAddr`].
///
/// [`V4 variant of SocketAddr`]: enum.SocketAddr.html#variant.V4
pub fn from() {}
A more modern way to write it as of #43466 would be:
/// [`V4 variant of SocketAddr`]: SocketAddr::V4
@bors delegate=skade |
✌️ @skade can now approve this pull request |
The inline attribute should be removed, or at least discussed in a separate PR. Those should only be added given real-world benchmarks demonstrating need. |
These look good to me. I do agree about dropping |
Ping from triage @skade! This PR needs your review. |
@bors r+ rollup (Approving since these look good to me as well, in addition to the LGTM from @joshtriplett). |
📌 Commit 4ced4f3 has been approved by |
💡 This pull request was already approved, no need to approve it again.
|
📌 Commit 4ced4f3 has been approved by |
@bors r- This has caused several intralink errors in #54021 (comment).
|
📌 Commit 4ced4f3 has been approved by |
Hey guys I am still traveling, I will fix this when I am back, soon. |
…r=TimNN Add doc for impl From for Addr As part of issue rust-lang#51430 (cc @skade). The impl is very simple, let me know if we need to go into any details. Additionally, I added `#[inline]` for the conversion method, let me know if it is un-necessary or might break something.
Rollup of 9 pull requests Successful merges: - #53522 (Add doc for impl From for Addr) - #54097 (rustdoc: Remove namespace for keywords) - #54205 (Add treat-err-as-bug flag in rustdoc) - #54225 (Regression test for #53675.) - #54232 (add `-Z dont-buffer-diagnostics`) - #54273 (Suggest to change numeric literal instead of casting) - #54299 (Issue 54246) - #54311 (Remove README with now-out-of-date docs about docs.) - #54313 (OsStr: Document that it's not NUL terminated) Failed merges: r? @ghost
As part of issue #51430 (cc @skade).
The impl is very simple, let me know if we need to go into any details.
Additionally, I added
#[inline]
for the conversion method, let me know if it is un-necessary or might break something.