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.

Sign up for GitHub

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

.github/workflows: Deny cargo doc warnings in CI #2936

Merged
merged 7 commits into from
Oct 4, 2022
Merged

.github/workflows: Deny cargo doc warnings in CI #2936

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d4b13aa
Deny cargo doc warnings in the check-rustdoc-links CI job
ddimaria Sep 23, 2022
13007e7
Allow private_intra_doc_links in swarm, use automatic links in libp2p…
ddimaria Sep 23, 2022
3fec9a4
Merge branch 'master' into fix-cargo-doc-warnings
ddimaria Sep 27, 2022
040b913
Remove clippy allow and remove --document-private-items from CI
ddimaria Sep 30, 2022
a993229
Lift private documentation onto public documentation
ddimaria Sep 30, 2022
6534eaa
Merge branch 'master' into fix-cargo-doc-warnings
ddimaria Oct 3, 2022
5fb21bd
Merge branch 'master' into fix-cargo-doc-warnings
thomaseizinger Oct 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ jobs:
- uses: Swatinem/rust-cache@6720f05bc48b77f96918929a9019fb2203ff71f8 # v2.0.0

- name: Check rustdoc links
run: RUSTDOCFLAGS="--deny broken_intra_doc_links" cargo doc --verbose --workspace --no-deps --document-private-items --all-features
run: RUSTDOCFLAGS="--deny rustdoc::broken_intra_doc_links --deny warnings" cargo doc --verbose --workspace --no-deps --document-private-items --all-features

check-clippy:
runs-on: ubuntu-latest
Expand Down
8 changes: 4 additions & 4 deletions protocols/gossipsub/src/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ impl GossipsubConfig {
/// connected/trusted nodes. The default is false.
///
/// Note: Peer exchange is not implemented today, see
/// https://github.com/libp2p/rust-libp2p/issues/2398.
/// <https://github.com/libp2p/rust-libp2p/issues/2398>.
pub fn do_px(&self) -> bool {
self.do_px
}
Expand Down Expand Up @@ -334,7 +334,7 @@ impl GossipsubConfig {
/// Number of heartbeat ticks that specifcy the interval in which opportunistic grafting is
/// applied. Every `opportunistic_graft_ticks` we will attempt to select some high-scoring mesh
/// peers to replace lower-scoring ones, if the median score of our mesh peers falls below a
/// threshold (see https://godoc.org/github.com/libp2p/go-libp2p-pubsub#PeerScoreThresholds).
/// threshold (see <https://godoc.org/github.com/libp2p/go-libp2p-pubsub#PeerScoreThresholds>).
/// The default is 60.
pub fn opportunistic_graft_ticks(&self) -> u64 {
self.opportunistic_graft_ticks
Expand Down Expand Up @@ -650,7 +650,7 @@ impl GossipsubConfigBuilder {
/// connected/trusted nodes. The default is false.
///
/// Note: Peer exchange is not implemented today, see
/// https://github.com/libp2p/rust-libp2p/issues/2398.
/// <https://github.com/libp2p/rust-libp2p/issues/2398>.
pub fn do_px(&mut self) -> &mut Self {
self.config.do_px = true;
self
Expand Down Expand Up @@ -725,7 +725,7 @@ impl GossipsubConfigBuilder {
/// Number of heartbeat ticks that specifcy the interval in which opportunistic grafting is
/// applied. Every `opportunistic_graft_ticks` we will attempt to select some high-scoring mesh
/// peers to replace lower-scoring ones, if the median score of our mesh peers falls below a
/// threshold (see https://godoc.org/github.com/libp2p/go-libp2p-pubsub#PeerScoreThresholds).
/// threshold (see <https://godoc.org/github.com/libp2p/go-libp2p-pubsub#PeerScoreThresholds>).
/// The default is 60.
pub fn opportunistic_graft_ticks(&mut self, opportunistic_graft_ticks: u64) -> &mut Self {
self.config.opportunistic_graft_ticks = opportunistic_graft_ticks;
Expand Down
2 changes: 1 addition & 1 deletion protocols/gossipsub/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
//! # Overview
//!
//! *Note: The gossipsub protocol specifications
//! (https://github.com/libp2p/specs/tree/master/pubsub/gossipsub) provide an outline for the
//! (<https://github.com/libp2p/specs/tree/master/pubsub/gossipsub>) provide an outline for the
//! routing protocol. They should be consulted for further detail.*
//!
//! Gossipsub is a blend of meshsub for data and randomsub for mesh metadata. It provides bounded
Expand Down
2 changes: 1 addition & 1 deletion swarm/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
//! The [`ConnectionHandler`] trait defines how each active connection to a
//! remote should behave: how to handle incoming substreams, which protocols
//! are supported, when to open a new outbound substream, etc.
//!
#![allow(rustdoc::private_intra_doc_links)]
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure if it makes sense to allow this. Do the offending links work on docs.rs? Because if not, then there isn't really much of a point I think.

I'd be in favor of rewriting the docs instead.

@mxinden Thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

I'd be in favor of rewriting the docs instead.

I would be in favor of this as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The links show up in docs.rs, though the links do not work because the connection and connection/pool modules are private, so completely agree on the doc rewrite (assuming making connection and connection/pool public isn't a good option). Links to the offending documentation:

I can lift the doc comments from the referenced functions and replace the private links with them, though this is not DRY, so it adds to the doc-update burden. Thoughts @thomaseizinger ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest we simply remove the docs from the non-public items and put them on the public ones. In other words, inline the documentation we are currently referring to and deleting it from there!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is done, though I left the documentation in the private functions alone as it looked out of place with the other items documented. It may be out of scope for this PR, but it's worth considering changing /// to // documentation in private items.


mod connection;
mod registry;
Expand Down