Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(webtransport-websys): Change logs to debug level #5396

Merged
merged 9 commits into from
May 26, 2024

Conversation

oblique
Copy link
Contributor

@oblique oblique commented May 17, 2024

Description

When you enable Kademlia in a large network that uses multiple type of transports, then this log creates a very noisy console and makes meaningful logs hard to see.

We can also remove the log entirely if you prefer. Check: #4015 (comment), #4072, #4133

Notes & 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

Copy link
Contributor

mergify bot commented May 17, 2024

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

Copy link
Contributor

@zvolin zvolin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can confirm, it's really noisy

transports/webtransport-websys/CHANGELOG.md Outdated Show resolved Hide resolved
@oblique
Copy link
Contributor Author

oblique commented May 20, 2024

For some reason CI got stuck

Copy link
Member

@jxs jxs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this. Interesting cargo-semver-checks does not complain, in fact I'd be more comfortable releasing this as a new minor wdyt?

@oblique
Copy link
Contributor Author

oblique commented May 20, 2024

Do you mean 0.3.0? I'm ok with it.

@jxs
Copy link
Member

jxs commented May 20, 2024

Do you mean 0.3.0? I'm ok with it.

yeah exactly, ok thanks

@oblique
Copy link
Contributor Author

oblique commented May 20, 2024

Question: In that case we will need to wait for libp2p 0.54 to incorporate this and #5390 in our crate. Do you have an approximate date for the release? Do you work with release cycles?

@jxs
Copy link
Member

jxs commented May 22, 2024

Question: In that case we will need to wait for libp2p 0.54 to incorporate this and #5390 in our crate. Do you have an approximate date for the release? Do you work with release cycles?

we're waiting on #5389 to release a new libp2p version, but meanwhile I can release libp2p-webtransport-websys and I think you can then depend on it directly, wdyt?

@oblique
Copy link
Contributor Author

oblique commented May 22, 2024

Sounds good!

Copy link
Member

@jxs jxs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jxs jxs added the send-it label May 26, 2024
@mergify mergify bot merged commit b83d3ce into libp2p:master May 26, 2024
72 checks passed
@jxs
Copy link
Member

jxs commented May 26, 2024

@oblique released libp2p-webtransport-sys 0.3.0

TimTinkers pushed a commit to unattended-backpack/rust-libp2p that referenced this pull request Sep 14, 2024
When you enable Kademlia in a large network that uses multiple type of transports, then this log creates a very noisy console and makes meaningful logs hard to see.

We can also remove the log entirely if you prefer. Check: libp2p#4015 (comment), libp2p#4072, libp2p#4133

Pull-Request: libp2p#5396.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants