swarm/src/behaviour: Deprecate NetworkBehaviourEventProcess

[mxinden]

Description

In preparation for https://github.com/libp2p/rust-libp2p/pull/2751.

Change checklist

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

[mxinden]

Change default for event_process to true instead of false. User that would like to keep using it would have to explicitly add #[event_process=false] rather than it being the default

I think there is a confusion here. Today, by default, event_process is false and thereby the derive macro uses the out_event mechanism instead of the NetworkBehaviourEventProcess mechanism. See https://github.com/libp2p/rust-libp2p/pull/2214.

https://github.com/libp2p/rust-libp2p/blob/eaf3f3a7fb4aba228ebd15366ab79b64cf37832c/swarm-derive/src/lib.rs#L73-L75

Am I missing something @elenaf9?

[elenaf9]

Change default for event_process to true instead of false. User that would like to keep using it would have to explicitly add #[event_process=false] rather than it being the default

I think there is a confusion here. Today, by default, event_process is false and thereby the derive macro uses the out_event mechanism instead of the NetworkBehaviourEventProcess mechanism. See #2214.

https://github.com/libp2p/rust-libp2p/blob/eaf3f3a7fb4aba228ebd15366ab79b64cf37832c/swarm-derive/src/lib.rs#L73-L75

Am I missing something @elenaf9?

You are right, I mixed it up what event_process=false means. But since default event_process=false already means that out_event is used it makes it even easier. So to rephrase my idea above: Instead of just deprecating NetworkBehaviourEventProcess, in my opinion the first release should already include the changes in the proc macro (and the changes in our examples and other behaviours, so effectively everything from #2751), just that we don't remove the support for event_process = true and NetworkBehaviourEventProcess yet. For users that want to stick with the deprecated event_process nothing would change. The benefit would be that users that do want to switch can then already use the generated enum. Any reason why this would not make sense?

[thomaseizinger]

I think that is a really good idea actually!

[mxinden]

@elenaf9 @thomaseizinger with https://github.com/libp2p/rust-libp2p/pull/2792 merged, would you mind taking another look?

[elenaf9]

One more thing: I think if we are deprecating NetworkBehaviourEventProcess we should also already change our examples & tests to not use it anymore (examples/distributed-key-value-store.rs, examples/chat.rs, ...). Can be done in a separate PR, but imo it should be part of the same release.

[mxinden]

One more thing: I think if we are deprecating NetworkBehaviourEventProcess we should also already change our examples & tests to not use it anymore (examples/distributed-key-value-store.rs, examples/chat.rs, ...). Can be done in a separate PR, but imo it should be part of the same release.

That is a good point. https://github.com/libp2p/rust-libp2p/pull/2784/commits/0f3ef691ff1499c3fb85f96c21a26b44fea28e23 does the change.

[mxinden]

As far as I can tell all comments are addressed and everyone approved. Thus I am merging here. Happy to do any follow-ups in case I missed something.

As always, thanks for all the input @elenaf9 and @thomaseizinger!