-
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
Add doc for impl From for Addr #53522
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.