Skip to content
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

Fix Rust docs build #1271

Merged
merged 4 commits into from
Dec 18, 2023
Merged

Fix Rust docs build #1271

merged 4 commits into from
Dec 18, 2023

Conversation

ibc
Copy link
Member

@ibc ibc commented Dec 18, 2023

Fixes #1270

@ibc ibc requested a review from nazar-pc December 18, 2023 21:16
@ibc
Copy link
Member Author

ibc commented Dec 18, 2023

Just wondering if cargo doc should be added to CI, I'm adding it.

@nazar-pc
Copy link
Collaborator

nazar-pc commented Dec 18, 2023

That alone will not fix this particular issue, DOCS_RS is an environment variable only set on workers that build documentation on docs.rs, you can set that environment variable in CI though.

I'd recommend this (+DOCS_RS): https://github.com/subspace/subspace/blob/1b508553239dec73b88872146c8df68f788bbfd2/.github/workflows/rust.yml#L134-L137

@ibc
Copy link
Member Author

ibc commented Dec 18, 2023

Ok, how good is this?

DOC_RS=true cargo doc
warning: unresolved link to `WebRtcServer`
  --> rust/src/data_structures.rs:48:43
   |
48 | /// Listening protocol, IP and port for [`WebRtcServer`] to listen on.
   |                                           ^^^^^^^^^^^^ no item named `WebRtcServer` in scope
   |
   = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
   = note: `#[warn(rustdoc::broken_intra_doc_links)]` on by default

warning: unresolved link to `crate::data_consumer::DataConsumer::on_data_producer_resume`
   --> rust/src/router/data_producer.rs:474:57
    |
474 |     /// Calls [`DataConsumer::on_data_producer_resume`](crate::data_consumer::DataConsumer::on_data_producer_resume)
    |                                                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the enum `DataConsumer` has no variant or associated item named `on_data_producer_resume`

warning: `mediasoup` (lib doc) generated 2 warnings
    Finished dev [unoptimized + debuginfo] target(s) in 0.34s


echo $?
0

@nazar-pc
Copy link
Collaborator

Yeah, you'll have to fix warnings now. Those are actual links to other data structures. You can replace

[``]

with just

``

So it is not a link anymore. Or make it a proper link like in a second warning, but make sure path to data structure is correct.

@ibc
Copy link
Member Author

ibc commented Dec 18, 2023

Wow, I've found a bug. I mean, docs did.

@ibc
Copy link
Member Author

ibc commented Dec 18, 2023

How is possible that this fix (I didn't change ANYTHING else) doesn't make cargo build to fail?

     /// Callback is called when the associated data producer is resumed.
-    pub fn on_producer_resume<F: Fn() + Send + Sync + 'static>(&self, callback: F) -> HandlerId {
+    pub fn on_data_producer_resume<F: Fn() + Send + Sync + 'static>(&self, callback: F) -> HandlerId {
         self.inner()
             .handlers
             .data_producer_resume

@nazar-pc
Copy link
Collaborator

It is just a public method name, could be anything 🤷‍♂️

@ibc
Copy link
Member Author

ibc commented Dec 18, 2023

cargo is wrong here:

DOC_RS=1 RUSTDOCFLAGS="-D rustdoc::broken-intra-doc-links -D rustdoc::private_intra_doc_links" cargo doc --locked --all --no-deps --lib

 Documenting mediasoup v0.13.0 (/Users/ibc/src/v3-mediasoup/rust)
error: unresolved link to `WebRtcServer`
  --> rust/src/data_structures.rs:48:59
   |
48 | ...`WebRtcServer`][`WebRtcServer`](crate::webrtc_server::WebRtcServer) to listen on.
   |                     ^^^^^^^^^^^^ no item named `WebRtcServer` in scope
   |
   = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
   = note: requested on the command line with `-D rustdoc::broken-intra-doc-links`

error: could not document `mediasoup`

@nazar-pc
Copy link
Collaborator

It isn't, looks like you have this:

[WebRtcServer]WebRtcServer

Instead of this:

WebRtcServer

@ibc
Copy link
Member Author

ibc commented Dec 18, 2023

OMG

@ibc
Copy link
Member Author

ibc commented Dec 18, 2023

PR ready. Yes, an unrelated change in Cargo.lock but I'm happy.

Copy link
Collaborator

@nazar-pc nazar-pc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And cargo fmt will not hurt

rust/src/router/data_consumer.rs Outdated Show resolved Hide resolved
@ibc ibc merged commit c5eb1b4 into v3 Dec 18, 2023
28 checks passed
@ibc ibc deleted the fix-rust-docs branch December 18, 2023 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Rust docs for 0.13 failed to build
2 participants