protocols/ping: Properly deprecate types with Ping prefix

[thomaseizinger]

Description

The deprecation we tried to do in #2215 doesn't work but defining type aliases works :)

Links to any relevant issues

• All crates should have a consistent API & naming #2217
• Revise naming of libp2p-ping symbols #2215

Open Questions

Change checklist

• I have performed a self-review of my own code
• I have made corresponding changes to the documentation
• I have added tests that prove my fix is effective or that my feature works
• A changelog entry has been made in the appropriate crates

[jxs]

Hi Thomas, when doing #2927 noticed there are still some references to the deprecated types in the docs, and some types with Ping prefix, should they be addressed?

(sorry for doing it like this, github still doesn't support reviewing on unchanged files nor unchanged code)

docs:

rust-libp2p/protocols/ping/src/lib.rs

Line 29 in a772e5a

//! The [`Ping`] struct implements the [`NetworkBehaviour`] trait. When used with a [`Swarm`],

rust-libp2p/protocols/ping/src/lib.rs

Line 34 in a772e5a

//! The `Ping` network behaviour produces [`PingEvent`]s, which may be consumed from the `Swarm`

rust-libp2p/protocols/ping/src/lib.rs

Line 38 in a772e5a

//! > by default, see [`PingConfig::with_keep_alive`] for changing this behaviour.

rust-libp2p/protocols/ping/src/handler.rs

Line 59 in a772e5a

/// Creates a new `PingConfig` with the following default settings:

rust-libp2p/protocols/ping/src/handler.rs

Line 211 in a772e5a

/// Builds a new `PingHandler` with the given configuration.

structs:

rust-libp2p/protocols/ping/src/handler.rs

Line 388 in a772e5a

enum PingState {

rust-libp2p/protocols/ping/src/handler.rs

Line 384 in a772e5a

type PingFuture = BoxFuture<'static, Result<(NegotiatedSubstream, Duration), io::Error>>;

[elenaf9]

What do you think about just removing the deprecated types? They have been marked as deprecated for quite some time now, and upgrading to the new names should be easy.

[thomaseizinger]

What do you think about just removing the deprecated types? They have been marked as deprecated for quite some time now, and upgrading to the new names should be easy.

They have been incorrectly marked as deprecated :)

[elenaf9]

What do you think about just removing the deprecated types? They have been marked as deprecated for quite some time now, and upgrading to the new names should be easy.

They have been incorrectly marked as deprecated :)

Incorrectly in the sense that users that were still using them did not get a warning? Does the deprecated macro not work on re-exports?

[jxs]

What do you think about just removing the deprecated types? They have been marked as deprecated for quite some time now, and upgrading to the new names should be easy.

They have been incorrectly marked as deprecated :)

Incorrectly in the sense that users that were still using them did not get a warning? Does the deprecated macro not work on re-exports?

Hi Elena, yeah exactly. See #2927 (comment)

[mxinden]

Looks good to me. Thanks for making it easier for users to upgrade.

If I am not mistaken, this is missing the suggestions by @jxs. Otherwise looks good to me.

[thomaseizinger]

If I am not mistaken, this is missing the suggestions by @jxs. Otherwise looks good to me.

Yep! Unfortunately, rustdoc doesn't flag links to deprecated items as deprecated. Grml.

[thomaseizinger]

Okay, I went across the repo again with fulltext search and found quite a few more usages.

I also realised that our CI doesn't check for warnings in examples! I'll see to fix that too.

[thomaseizinger]

I also realised that our CI doesn't check for warnings in examples! I'll see to fix that too.

Done: #2949

[jxs]

Okay, I went across the repo again with fulltext search and found quite a few more usages.

I also realised that our CI doesn't check for warnings in examples! I'll see to fix that too.

awesome, LGTM!
Did you see the references to PingState and PingFuture ? PingState can be renamed to just State as it isn't shared outside of the module. though PingFuture and PongFuture probably there isn't a better wording for them.

[thomaseizinger]

Okay, I went across the repo again with fulltext search and found quite a few more usages.
I also realised that our CI doesn't check for warnings in examples! I'll see to fix that too.

awesome, LGTM! Did you see the references to PingState and PingFuture ? PingState can be renamed to just State as it isn't shared outside of the module. though PingFuture and PongFuture probably there isn't a better wording for them.

I guess PingState could be renamed yeah. I think public API types are the most important. Everything else we can any time :)