refactor: move examples to common location #3509

yellowred · 2023-02-27T04:59:31Z

Description

Refactor examples into separate binary crates.

Links to any relevant issues

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

thomaseizinger

Excellent start. I am pleased that Git recognizes these as "moved". That should make this fairly easy to review and merge.

examples/chat/Cargo.toml

examples/README.md

examples/ipfs-kad/Cargo.toml

examples/rendezvous-register/Cargo.toml

yellowred · 2023-03-02T22:33:22Z

Hi @thomaseizinger, I have moved all the examples to their crates. chat-tokio, gossipsub-chat and mdns-passive-discovery are excluded since they duplicate functionality from other examples. Let me know if you want them back and also if you would like to add other examples. I would add QUIC as OrTransport somewhere, but as you said we need a model example that has all the transports showcased.

EDIT: Still have to resolve some naming collisions in binaries.

thomaseizinger · 2023-03-02T22:54:07Z

Awesome, thanks a lot for this!

Ping me once you've got CI passing or need help with it :)

thomaseizinger

Thanks for pushing this forward!

I made some comments, most of them a pretty minor. I really like the direction of this :)

thomaseizinger · 2023-03-03T01:50:27Z

Cargo.toml

-[[example]]
-name = "chat"
-required-features = ["full"]
-
-[[example]]
-name = "chat-tokio"
-required-features = ["full"]
-
-[[example]]
-name = "file-sharing"
-required-features = ["full"]
-
-[[example]]
-name = "gossipsub-chat"
-required-features = ["full"]
-
-[[example]]
-name = "ipfs-private"
-required-features = ["full"]
-
-[[example]]
-name = "ipfs-kad"
-required-features = ["full"]
-
-[[example]]
-name = "ping"
-required-features = ["full"]
-
-[[example]]
-name = "mdns-passive-discovery"
-required-features = ["full"]
-
-[[example]]
-name = "distributed-key-value-store"
-required-features = ["full"]


thomaseizinger · 2023-03-03T01:51:25Z

examples/dcutr/Cargo.toml

+env_logger = "0.10.0"
+futures = "0.3.26"
+futures-timer = "3.0"
+libp2p = { path = "../../", features=["async-std", "dns", "dcutr", "identify", "macros", "mplex", "noise", "ping", "relay", "rendezvous", "tcp", "tokio", "yamux"] }


Suggested change

libp2p = { path = "../../", features=["async-std", "dns", "dcutr", "identify", "macros", "mplex", "noise", "ping", "relay", "rendezvous", "tcp", "tokio", "yamux"] }

libp2p = { path = "../../", features = ["async-std", "dns", "dcutr", "identify", "macros", "mplex", "noise", "ping", "relay", "rendezvous", "tcp", "tokio", "yamux"] }

Formatting nit.

thomaseizinger · 2023-03-03T01:52:00Z

examples/dcutr/src/main.rs

-        event_process = false,
-        prelude = "libp2p_swarm::derive_prelude"
-    )]
+    #[behaviour(out_event = "Event", event_process = false, prelude = "derive_prelude")]


Suggested change

#[behaviour(out_event = "Event", event_process = false, prelude = "derive_prelude")]

#[behaviour(out_event = "Event", event_process = false)]

When depending on the libp2p crate, this is not needed!

thomaseizinger · 2023-03-03T01:53:44Z

examples/ipfs-kad/Cargo.toml

+env_logger = "0.10"
+futures = "0.3.26"
+libp2p = { path = "../../", features=["async-std", "dns", "kad",  "mplex", "noise", "tcp", "websocket", "yamux"] }
+multiaddr = { version = "0.17.0" }


We should be able to import multiaddr through libp2p::core::multiaddr. Why is the separate dependency needed?

thomaseizinger · 2023-03-03T01:54:38Z

examples/rendezvous/Cargo.toml

+name = "rzv-discover"
+path = "src/bin/discover.rs"


I'd be in favor of just renaming the files and rely on auto-discover of the binaries. Convention over configuration.

thomaseizinger · 2023-03-03T01:59:48Z

examples/chat/Cargo.toml

@@ -0,0 +1,12 @@
+[package]
+name = "chat"


Would it make sense to append example to these like chat-example? Then it would show up as "Building chat-example" in the cargo logs which I'd mildly prefer.

thomaseizinger · 2023-03-03T02:01:33Z

examples/rendezvous/src/bin/register.rs

+        libp2p::tcp::tokio::Transport::default()
            .upgrade(Version::V1)
-            .authenticate(libp2p_noise::NoiseAuthenticated::xx(&identity).unwrap())
-            .multiplex(libp2p_yamux::YamuxConfig::default())
+            .authenticate(libp2p::noise::NoiseAuthenticated::xx(&key_pair).unwrap())
+            .multiplex(libp2p::yamux::YamuxConfig::default())


We are in the process of apply more consistent naming to all our crates to make usage easier and clearer. Can we import the submodules here please like use libp2p::tcp; and then say tcp::tokio::Transport?

thomaseizinger · 2023-03-03T02:03:13Z

examples/rendezvous/src/bin/register.rs

+        void::unreachable(event)
+    }
+}
+
 #[derive(NetworkBehaviour)]
 #[behaviour(
    out_event = "MyEvent",


In a recent version of the NetworkBehaviour derive, we add the ability to auto-generate this enum. I think our examples should use that as we consider it the recommended way of using the derive. Can you adapt the examples please? The only thing that will needed changing is that the generated enum will be called MyBehaviourEvent.

thomaseizinger · 2023-03-03T02:03:58Z

examples/pingpong/Cargo.toml

@@ -0,0 +1,12 @@
+[package]
+name = "pingpong"


Suggested change

name = "pingpong"

name = "ping-example"

examples/file-sharing/src/main.rs

thomaseizinger

Nice, thank you for that! A few more minor comments.

examples/rendezvous/Cargo.toml

thomaseizinger · 2023-03-04T08:37:54Z

examples/rendezvous/src/bin/rzv-identify.rs

    let mut swarm = Swarm::with_tokio_executor(
-        libp2p_tcp::tokio::Transport::default()
+        libp2p::tcp::tokio::Transport::default()


Suggested change

libp2p::tcp::tokio::Transport::default()

tcp::tokio::Transport::default()

Nit: Could we also align these please? i.e. add use libp2p::tcp.

examples/rendezvous/src/bin/rzv-identify.rs

thomaseizinger

Two more small requests, sorry for the back and forth!

thomaseizinger · 2023-03-04T21:28:18Z

examples/rendezvous/Cargo.toml

@@ -0,0 +1,14 @@
+[package]
+name = "rendezvous"


Can we consistently apply the naming of appending -example here? Thanks!

thomaseizinger · 2023-03-04T21:30:55Z

examples/rendezvous/src/bin/rzv-identify.rs

+            .authenticate(libp2p::noise::NoiseAuthenticated::xx(&key_pair).unwrap())
+            .multiplex(libp2p::yamux::YamuxConfig::default())


Same with this topic here. I didn't explicitly comment on all lines but let's do this consistently across all examples and use yamux:: etc in the code and use libp2p::yamux at the top. (For all libp2p modules: noise, yamux, etc)

sorry, missed it

mergify · 2023-03-06T11:51:19Z

This pull request has merge conflicts. Could you please resolve them @yellowred? 🙏

mergify · 2023-03-07T13:34:36Z

This pull request has merge conflicts. Could you please resolve them @yellowred? 🙏

``` error: document output filename collision The bin `ping` in package `ping v0.1.0 (/home/runner/work/rust-libp2p/rust-libp2p/examples/ping)` has the same name as the bin `ping` in package `interop-tests v0.1.0 (/home/runner/work/rust-libp2p/rust-libp2p/interop-tests)`. ```

yellowred · 2023-03-08T06:41:06Z

@thomaseizinger there is one tiny check fails, not sure why, otherwise should be ok?

thomaseizinger · 2023-03-08T08:12:24Z

@thomaseizinger there is one tiny check fails, not sure why, otherwise should be ok?

A quick investigation leads to this: clap-rs/clap#4733

Seems like we need to update clap for this to be resolved fully. We do however don't require beta clippy to pass so this doesn't block this PR.

thomaseizinger

Thank you for tackling this, I am convinced this makes our repository a bit more approachable :)

thomaseizinger reviewed Feb 27, 2023

View reviewed changes

examples/chat/Cargo.toml Outdated Show resolved Hide resolved

examples/chat/Cargo.toml Outdated Show resolved Hide resolved

examples/README.md Outdated Show resolved Hide resolved

examples/README.md Show resolved Hide resolved

thomaseizinger reviewed Feb 28, 2023

View reviewed changes

examples/ipfs-kad/Cargo.toml Outdated Show resolved Hide resolved

thomaseizinger reviewed Feb 28, 2023

View reviewed changes

examples/rendezvous-register/Cargo.toml Outdated Show resolved Hide resolved

yellowred force-pushed the examples_refactoring branch from b340780 to e50dc79 Compare March 2, 2023 22:02

yellowred marked this pull request as ready for review March 2, 2023 22:03

yellowred changed the title ~~Draft: Refactor examples~~ Refactor examples Mar 2, 2023

yellowred changed the title ~~Refactor examples~~ refactor: examples Mar 2, 2023

thomaseizinger reviewed Mar 3, 2023

View reviewed changes

thomaseizinger changed the title ~~refactor: examples~~ refactor: move examples to common location Mar 3, 2023

yellowred requested a review from thomaseizinger March 3, 2023 22:03

thomaseizinger reviewed Mar 4, 2023

View reviewed changes

yellowred force-pushed the examples_refactoring branch from eef07e9 to 549e781 Compare March 6, 2023 23:25

yellowred requested a review from thomaseizinger March 7, 2023 00:00

yellowred added 12 commits March 7, 2023 09:43

Moved the Chat example into example crate.

f919806

Remove unnecessary deps

ee6d2fb

Replace chat example with gossipsub version.

e9d65a2

Moving more examples to separate binary crates.

7255c8c

Move Rendezvous protocol examples into separate binary crates.

5a1030c

Gather Rendezvous examples in a single crate.

bfe580c

Move a DCUtR example into a separate crate.

0f48838

No publishing for dcutr example

4a53d80

Move Identify example to a separate crate.

e535d68

Added new lines into Cargo.toml files in examples.

5216f39

cleanup

3e11c30

yellowred added 11 commits March 7, 2023 09:44

Fix IPFS integration tests.

7ed9a1c

Use namesapced binaries to avoid name collision in CI.

8112a24

Fix the docs markup

30006ec

Improved naming and formatting

db91624

Change to relative references for easier refactoring later

c0ae059

Use an Event derived by macro rather than defining it myself.

b84abe5

More improvements in naming

85363e7

Split network module into a separate file.

3264a6b

More references use relative location.

7b4bc75

Use relative path when importing.

3839920

cleanup

6568b2d

yellowred force-pushed the examples_refactoring branch from 549e781 to 6568b2d Compare March 7, 2023 17:46

thomaseizinger approved these changes Mar 8, 2023

View reviewed changes

thomaseizinger added the send-it label Mar 8, 2023

mergify bot merged commit b63e05d into libp2p:master Mar 8, 2023

yellowred deleted the examples_refactoring branch March 9, 2023 05:21

helloimalemur mentioned this pull request Mar 20, 2023

fix: Added Example 'cargo run --example ping' #3634

Closed

2 tasks

thomaseizinger mentioned this pull request Sep 12, 2023

Add more examples of getting and putting records in the IPFS DHT #2263

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: move examples to common location #3509

refactor: move examples to common location #3509

yellowred commented Feb 27, 2023 •

edited by thomaseizinger

Loading

thomaseizinger left a comment

yellowred commented Mar 2, 2023 •

edited

Loading

thomaseizinger commented Mar 2, 2023

thomaseizinger left a comment

thomaseizinger Mar 3, 2023

thomaseizinger Mar 3, 2023

thomaseizinger Mar 3, 2023

thomaseizinger Mar 3, 2023

thomaseizinger Mar 3, 2023

thomaseizinger Mar 3, 2023

thomaseizinger Mar 3, 2023

thomaseizinger Mar 3, 2023

thomaseizinger Mar 3, 2023

thomaseizinger left a comment

thomaseizinger Mar 4, 2023

thomaseizinger left a comment

thomaseizinger Mar 4, 2023

thomaseizinger Mar 4, 2023

yellowred Mar 6, 2023

mergify bot commented Mar 6, 2023

mergify bot commented Mar 7, 2023

yellowred commented Mar 8, 2023

thomaseizinger commented Mar 8, 2023

thomaseizinger left a comment

	libp2p = { path = "../../", features=["async-std", "dns", "dcutr", "identify", "macros", "mplex", "noise", "ping", "relay", "rendezvous", "tcp", "tokio", "yamux"] }
	libp2p = { path = "../../", features = ["async-std", "dns", "dcutr", "identify", "macros", "mplex", "noise", "ping", "relay", "rendezvous", "tcp", "tokio", "yamux"] }

	#[behaviour(out_event = "Event", event_process = false, prelude = "derive_prelude")]
	#[behaviour(out_event = "Event", event_process = false)]

	libp2p::tcp::tokio::Transport::default()
	tcp::tokio::Transport::default()

		.authenticate(libp2p::noise::NoiseAuthenticated::xx(&key_pair).unwrap())
		.multiplex(libp2p::yamux::YamuxConfig::default())

refactor: move examples to common location #3509

refactor: move examples to common location #3509

Conversation

yellowred commented Feb 27, 2023 • edited by thomaseizinger Loading

Description

Links to any relevant issues

Change checklist

thomaseizinger left a comment

Choose a reason for hiding this comment

yellowred commented Mar 2, 2023 • edited Loading

thomaseizinger commented Mar 2, 2023

thomaseizinger left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thomaseizinger left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thomaseizinger left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mergify bot commented Mar 6, 2023

mergify bot commented Mar 7, 2023

yellowred commented Mar 8, 2023

thomaseizinger commented Mar 8, 2023

thomaseizinger left a comment

Choose a reason for hiding this comment

yellowred commented Feb 27, 2023 •

edited by thomaseizinger

Loading

yellowred commented Mar 2, 2023 •

edited

Loading