First change to validate

[postmeback]

Issue Addressed

Fixes #4499

Proposed Changes

Remove the implementation of null_logger, else implement test_logger.

null_logger can be found common/task_executor/src/test_utils.rs

test_logger can be found common/logging/src

Additional Info

I have done only one change as of now. Please review and let me know, whether I am on a right path. Then, I will do changes in other places.

[ackintosh]

Could you please change the target branch from stable to unstable?

[postmeback]

Target branch changed from stable to unstable.

Can you review my changes, and let me know whether I am on the right path or not?

[paulhauner]

Can you review my changes, and let me know whether I am on the right path or not?

Yep you're on the right path, feel free to proceed!

[postmeback]

Changes done, Requesting for review.

Open for suggestions.

[postmeback]

Requesting for review

[paulhauner]

Heyo, I tried to fix the merge conflicts but I ran afoul of the formatter sorry. Could you please do git pull && cargo fmt and the commit+push the result?

[postmeback]

Heyo, I tried to fix the merge conflicts but I ran afoul of the formatter sorry. Could you please do git pull && cargo fmt and the commit+push the result?

Sure, let me try.

[postmeback]

I did git pull and cargo fmt.

Two files got changed in my branch. Shall I commit the cargo.lock ?

[paulhauner]

Two files got changed in my branch. Shall I commit the cargo.lock ?

I'm not sure, but let's give it a go. Try committing both files and we can see what happens :)

[postmeback]

Okay.

[postmeback]

Done.

[paulhauner]

Sorry @postmeback, it looks like committing the Cargo.lock wasn't the right thing to do. To fix it, I'd run:

git checkout unstable Cargo.lock
cargo check
git add Cargo.lock
git commit -m "Refresh Cargo.lock"
git push

[postmeback]

So, now it is ok to go forward ?

[postmeback]

The work-in-progress tab can be removed.

[paulhauner]

I pushed a fix to the Cargo.lock myself, so that's resolved now. However I can see that tests are failing and there are also instances of null_logger still remaining:

grep -rnw '.' -e 'null_logger' --exclude-dir 'target'
./common/task_executor/src/test_utils.rs:29:        let log = null_logger().unwrap();
./common/task_executor/src/test_utils.rs:70:pub fn null_logger() -> Result<Logger, String> {
./common/logging/src/lib.rs:236:            .expect("Should build null_logger")
./beacon_node/network/src/network_beacon_processor/mod.rs:14:use environment::null_logger;
./beacon_node/network/src/network_beacon_processor/mod.rs:553:        let log = null_logger().unwrap();

[paulhauner]

My advice would be to delete the null_logger() definition in ./common/task_executor/src/test_utils.rs:70: and then go about making sure that you can get both of these commands to run without error:

1. make lint
2. make udeps

That should be a surefire way of ensuring there are no more uses of null_logger and it would be a good start to producing something that will pass CI ☺️

[postmeback]

Okay, will sure try. Thanks for the suggestion. ☺️

[dapplion]

Pinging author @postmeback

[dapplion]

Closing for inactivity. Thank you @postmeback for the contribution! Please feel free to pick other good first issues if you want to contribute again

[postmeback]

Sorry was busy IRL. 😞