-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Documentation for impl From for AtomicBool and other Atomic types #53506
Conversation
r? @KodrAus (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 |
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 |
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.
Thanks @phungleson!
Looks like we need to fix up the concat!
call we're using.
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 |
Ping from triage @KodrAus! This PR needs your review. |
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.
Thanks for your patience @phungleson.
I've just left a few comments.
Ping from triage @phungleson: Some changes have been requested to your PR. |
I have addressed the comments @KodrAus |
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 |
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 |
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 |
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 |
Ah sorry @phungleson, it looks like rustdoc can't resolve these references automatically after all. I thought we could resolve links automatically for any type that was in scope, but we can only resolve links for types that are in the same module. We're getting an error building the docs saying those links can't be resolved, so it looks like we'll have to go back to your original idea of building the link using the |
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 |
Ping from triage @phungleson, what is the status of this PR? |
Ping form triage @phungleson: Could you give us an update on the status of this PR? |
The way we refers to other places seem to be changing quite a bit and it is different from area to area. Like this PR https://github.com/rust-lang/rust/pull/53507/files looks like we can just write the name, but in this PR, looks like we still have to specify the name at the end of the docs area. Do we have a plan to make it consistent? not having to specify anything is the best. |
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 |
@Mark-Simulacrum Thanks for the ping! I have had a look locally and the links do work, and I'm not getting any errors out of |
Do we perhaps want to drop the links to bool/i32? It seems like that might allow this PR to move forward and links can be added separately. Also cc @QuietMisdreavus, perhaps you can help diagnose the link check failures here? |
Ping from triage @phungleson: It looks like a potential way forward for this PR has been suggested (just dropping the problematic links for now). |
Ping from triage... @phungleson, any thoughts on moving this PR ahead? |
From the log I can see the links are broken for all the number formats, so do you guys think if removing only The issue seems to be that the number are missing in some test environments. |
The links are broken because they're appearing on the page(s) for Using intra-doc links to link to primitives likely won't work either, because we've had problems trying to use those to link to primitive docs from core. (It requires being able to load std to have those available, which creates exotic linking problems with duplicate libcore libraries.) You'll probably have to ditch the links. |
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 |
Success! 🎉 @phungleson Would you like to give these commits a squash and I'll get this one merged :) |
All done guys, this is probably the longest simplest PR ever 😃 |
Thanks for your patience in working through this! Most of the time it's a bit more obvious why the build fails. @bors r+ rollup |
📌 Commit 94c1c73 has been approved by |
…KodrAus Documentation for impl From for AtomicBool and other Atomic types As part of issue rust-lang#51430 (cc @skade). The impl is very simple, so not sure if we need to go into any details.
…KodrAus Documentation for impl From for AtomicBool and other Atomic types As part of issue rust-lang#51430 (cc @skade). The impl is very simple, so not sure if we need to go into any details.
…KodrAus Documentation for impl From for AtomicBool and other Atomic types As part of issue rust-lang#51430 (cc @skade). The impl is very simple, so not sure if we need to go into any details.
Rollup of 20 pull requests Successful merges: - #53506 (Documentation for impl From for AtomicBool and other Atomic types) - #56343 (Remove not used mod) - #56439 (Clearer error message for dead assign) - #56640 (Add FreeBSD unsigned char platforms to std::os::raw) - #56648 (Fix BTreeMap UB) - #56672 (Document time of back operations of a Linked List) - #56706 (Make `const unsafe fn` bodies `unsafe`) - #56742 (infer: remove Box from a returned Iterator) - #56761 (Suggest using `.display()` when trying to print a `Path`) - #56781 (Update LLVM submodule) - #56789 (rustc: Add an unstable `simd_select_bitmask` intrinsic) - #56790 (Make RValue::Discriminant a normal Shallow read) - #56793 (rustdoc: look for comments when scraping attributes/crates from doctests) - #56826 (rustc: Add the `cmpxchg16b` target feature on x86/x86_64) - #56832 (std: Use `rustc_demangle` from crates.io) - #56844 (Improve CSS rule) - #56850 (Fixed issue with using `Self` ctor in typedefs) - #56855 (Remove u8 cttz hack) - #56857 (Fix a small mistake regarding NaNs in a deprecation message) - #56858 (Fix doc of `std::fs::canonicalize`) Failed merges: - #56741 (treat ref-to-raw cast like a reborrow: do a special kind of retag) r? @ghost
As part of issue #51430 (cc @skade).
The impl is very simple, so not sure if we need to go into any details.