Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

docs: update references to NetworkConfiguration::extra_sets #7386

Merged
merged 2 commits into from
Jun 16, 2023

Conversation

altaua
Copy link
Contributor

@altaua altaua commented Jun 16, 2023

Since paritytech/substrate#14080, this struct field no longer exists, now the add_notification_protocol() function of
sc_network::config::FullNetworkConfiguration is used.

Also neuter the doc links for now; rustdoc can't resolve them (presumably because sc_network::config isn't in scope, though weirdly enough even spelling the link out as
[`FullNetworkConfiguration`](struct@sc_network::config::FullNetworkConfiguration) doesn't work?). Normally this wouldn't be an issue and rustdoc would just not generate links, but rust 1.70 has a bug that completely crashes rustdoc in this case.

Required for https://github.com/paritytech/ci_cd/issues/816

Since paritytech/substrate#14080, this struct field no longer exists,
now the `add_notification_protocol()` function of
`sc_network::config::FullNetworkConfiguration` is used.

Also neuter the doc links for now; rustdoc can't resolve them
(presumably because sc_network::config isn't in scope, though weirdly
enough even spelling the link out as
``[`FullNetworkConfiguration`](struct@sc_network::config::FullNetworkConfiguration)``
doesn't work?). Normally this wouldn't be an issue and rustdoc would
just not generate links, but rust 1.70 has a bug that completely crashes
rustdoc in this case.
@altaua altaua added A2-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Jun 16, 2023
@altaua altaua self-assigned this Jun 16, 2023
@@ -34,7 +34,7 @@ use polkadot_node_network_protocol::{

/// Peer set info for network initialization.
///
/// To be added to [`NetworkConfiguration::extra_sets`].
/// To be added to [`FullNetworkConfiguration`]().
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// To be added to [`FullNetworkConfiguration`]().
/// To be passed to [`FullNetworkConfiguration::add_notification_protocol`](sc_network::config::FullNetworkConfiguration::add_notification_protocol).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've applied your suggestions. I'm still leaving the () link part at the end empty though; rustdoc doesn't seem to be able to resolve sc_network::config::FullNetworkConfiguration::add_notification_protocol, and that's triggering the aforementioned crashing bug in 1.70.

Copy link
Member

Choose a reason for hiding this comment

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

Then this should be reported upstream.

node/network/bridge/src/rx/mod.rs Outdated Show resolved Hide resolved
node/network/bridge/src/tx/mod.rs Outdated Show resolved Hide resolved
@altaua altaua merged commit cdc906f into master Jun 16, 2023
@altaua altaua deleted the mira/avoid-rustdoc-ice branch June 16, 2023 11:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A2-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants