Pull reviewers stats

Stats of the last 365 days for mediasoup:

	User	Total reviews	Time to review	Total comments
	ibc 🥇	67 ▀▀▀▀	2h 58m	173 ▀▀▀▀▀▀
	jmillan 🥈	56 ▀▀▀	15h 39m	55 ▀▀
	nazar-pc 🥉	29 ▀▀	2h 20m	41 ▀
	ggarber	4	3h 11m	3
	eli-schwartz	2	1h 34m	2
	fippo	1	8h 29m	2
	SteveMcFarlin	1	2d 8h 1m	3
	Hartigan	1	2d 5h 7m	6
	balazskreith	1	55d 2h 38m ▀▀▀▀▀▀▀▀▀	12

@nazar-pc I strongly think we should freeze Rust version in mediasoup-rust to avoid these unexpected and totally unrelated CI errors: https://github.com/versatica/mediasoup/actions/runs/4023390642/jobs/6914204443

Problem is that cargo clippy works for me in my computer. How should I update Rust? Via brew install rust? Or brew install cargo?

New version was out yesterday with new lints enabled by default: https://blog.rust-lang.org/2023/01/26/Rust-1.67.0.html
Updating can be done easily with rustup update.

@nazar-pc I need help here:

@nazar-pc I need help here:

Trying to apply this change:

--- a/rust/src/worker.rs
+++ b/rust/src/worker.rs
@@ -500,8 +500,7 @@ impl Inner {
                     Err(error) => Err(io::Error::new(
                         io::ErrorKind::Other,
                         format!(
-                            "unexpected first notification from worker [id:{}]: {:?}; error = {}",
-                            id, notification, error
+                            "unexpected first notification from worker [id:{id}]: {notification}; error = {error}"
                         ),
                     )),
                 };

I've tried this(as per output help):

then it complains again with:

It was {:?}, which is print debug version of the data, not regular ({}), so it should be {notification:?}:

                        format!(
                            "unexpected first notification from worker [id:{id}]: {notification:?}; \
                            error = {error}"
                        ),

When strings are longer than configured length formatter gives up formatting other code around it sometimes, so better splitting it into multiple lines explicitly in those cases.

UPD: Error message actually suggested :?, I guess you overlooked it due to 2 other variables.

Thanks @nazar-pc, pushed. Should be green now.

Rust is not happy with your skills today, sir. Use cargo clippy --all-targets at the root of the repo in case you run it under rust only

Did cargo clippy --all-targets ignore rust/tests folder because I executed it in rust folder? WHAT? How can be this language legal?

I don't think that was the case, tests are covered by that command.
CI just runs that command in the root of the repo and refuses to accept any warnings.
I think you're close though.

Really???

Only plain variables can be used, not expressions or property accesses like self.id. Let me fix those for you and push a commit into this branch.

All fixed in last commit.

This error didn't show locally: https://github.com/versatica/mediasoup/actions/runs/4024845039/jobs/6917332591

This error didn't show locally: https://github.com/versatica/mediasoup/actions/runs/4024845039/jobs/6917332591

Because it is platform-specific code. Unless you're on Windows, it is not compiled.

rust-toolchain.toml will make sure cargo downloads and uses desired version of Rust toolchain and components necessary for operation, including for CI to run successfully. There should be no guessing or mismatched tooling versions anymore.

rust-toolchain.toml will make sure cargo downloads and uses desired version of Rust toolchain and components necessary for operation, including for CI to run successfully. There should be no guessing or mismatched tooling versions anymore.

amazing

All 🟢