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

[non-breaking] deps(udp): make tracing optional and add optional log #1923

Merged
merged 3 commits into from
Jul 19, 2024

Conversation

mxinden
Copy link
Collaborator

@mxinden mxinden commented Jul 19, 2024

Alternative to #1921.

Implementation of @Ralith suggestion in #1921 (comment).

Not breaking quinn-udp. I.e. only bumping patch version to v0.5.3.

Copy link
Member

@djc djc left a comment

Choose a reason for hiding this comment

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

LGTM!

quinn-udp/Cargo.toml Outdated Show resolved Hide resolved
quinn-udp/Cargo.toml Outdated Show resolved Hide resolved
This commit makes the `tracing` dependency in `quinn-udp` optional, but enabled
by default. In additional it adds optional logging via the `log` crate, enabled
through the `direct-log` feature. `tracing` takes precedence over `native-log`.
@mxinden mxinden force-pushed the optional-tracing-non-breaking branch from 67b4325 to 46944db Compare July 19, 2024 15:07
Copy link
Collaborator

@Ralith Ralith left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@Ralith Ralith merged commit ff91ffe into quinn-rs:main Jul 19, 2024
8 checks passed
mxinden added a commit to mxinden/quinn that referenced this pull request Jul 22, 2024
Previously `quinn*` would provide the `log` feature to log events via `log` if
no `tracing` subscriber exists.

Later quinn-rs#1923 allowed `quinn-udp` to log via
`log` directly, making `tracing` an optional dependency. For that, it introduced
the `direct-log` feature, a workaround name in order to not introduce a breaking
change.

This commit cleans up the above, renaming the `log` feature to `tracing-log` and
the `direct-log` to `log`. This is a breaking change and thus `quinn-udp` is
bumped to `v0.6.0`.

See quinn-rs#1921 for the full history.
@mxinden
Copy link
Collaborator Author

mxinden commented Jul 22, 2024

Thank you @djc and @Ralith for the very quick help and fix here!

This is a great step forward for me. The only audits remaining for quinn-udp to land in Firefox is quinn-udp itself 🎉

Could you release quinn-udp v0.5.3 sometime this week?

@djc
Copy link
Member

djc commented Jul 22, 2024

Sure, want to submit a PR to bump the version number?

@mxinden
Copy link
Collaborator Author

mxinden commented Jul 22, 2024

@djc already done in this pull request:

https://github.com/quinn-rs/quinn/blob/main/quinn-udp/Cargo.toml#L3

@djc
Copy link
Member

djc commented Jul 22, 2024

Ah sorry, was looking at an outdated checkout. I'll just publish it now then!

@djc
Copy link
Member

djc commented Jul 22, 2024

  • Published quinn-udp v0.5.3 at registry crates-io
  • [new tag] quinn-udp-0.5.3 -> quinn-udp-0.5.3

@mxinden
Copy link
Collaborator Author

mxinden commented Jul 22, 2024

🙏 thanks again for the quick help @djc!

mxinden added a commit to mxinden/neqo that referenced this pull request Jul 23, 2024
quinn-rs/quinn#1923 made the `tracing` dependency
optional. In addition, when `direct-log` is enabled, it allows using `log`
instead of `tracing` for logging.
github-merge-queue bot pushed a commit to mozilla/neqo that referenced this pull request Jul 23, 2024
* deps(quinn-udp): move to workspace dependency

* deps(quinn-udp): bump to latest version

* deps(quinn-udp): enable direct-log feature

quinn-rs/quinn#1923 made the `tracing` dependency
optional. In addition, when `direct-log` is enabled, it allows using `log`
instead of `tracing` for logging.
github-merge-queue bot pushed a commit to mozilla/neqo that referenced this pull request Jul 23, 2024
* deps(quinn-udp): move to workspace dependency

* deps(quinn-udp): bump to latest version

* deps(quinn-udp): enable direct-log feature

quinn-rs/quinn#1923 made the `tracing` dependency
optional. In addition, when `direct-log` is enabled, it allows using `log`
instead of `tracing` for logging.
github-merge-queue bot pushed a commit to mozilla/neqo that referenced this pull request Jul 23, 2024
* deps(quinn-udp): move to workspace dependency

* deps(quinn-udp): bump to latest version

* deps(quinn-udp): enable direct-log feature

quinn-rs/quinn#1923 made the `tracing` dependency
optional. In addition, when `direct-log` is enabled, it allows using `log`
instead of `tracing` for logging.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants