Skip to content

Upgrade tracing-subscriber#2048

Merged
jneem merged 2 commits intolinebender:masterfrom
jplatte:tracing-bump
Nov 26, 2021
Merged

Upgrade tracing-subscriber#2048
jneem merged 2 commits intolinebender:masterfrom
jplatte:tracing-bump

Conversation

@jplatte
Copy link
Member

@jplatte jplatte commented Nov 20, 2021

I use tracing-subscriber directly in my project and don't like duplicate dependencies.

@ngugcx
Copy link
Contributor

ngugcx commented Nov 20, 2021

Is there any special reason why Druid depends on tracing?

@jplatte
Copy link
Member Author

jplatte commented Nov 20, 2021

It's a convenience thing, AppLauncher::log_to_console which uses tracing-subscriber on native targets and tracing-subscriber + tracing-wasm on wasm.

I wouldn't personally mind if it was instead removed though 🤷🏼

The crate was renamed and the latest release of test-env-log now raises
deprecation warnings.
@sjoshid
Copy link
Contributor

sjoshid commented Nov 21, 2021

Im assuming the problem here is that you are using version 0.3.2 in your project while druid uses version 0.2.15. So the PR makes druid use version 0.3.2. Now both use version 0.3.2 and problem solved.
But wouldnt you run into same issue if druid uses version > 0.3.2 in future?

@jplatte
Copy link
Member Author

jplatte commented Nov 21, 2021

There's not really a big problem here, it's just that build times get longer when you have duplicates of the same crate in the dependency tree. Yes of course druid should upgrade tracing-subscriber again in the future (when there's a breaking change release).

@sjoshid
Copy link
Contributor

sjoshid commented Nov 22, 2021

The point that I was trying to make was Druid is not going to update to latest and greatest version to satisfy a downstream client.
I empathize with your dependency issues but the real fix would be to completely remove it from Druid.

@jneem
Copy link
Member

jneem commented Nov 23, 2021

We can certainly discuss removing the dependency, but at least since it's there for now, we may as well keep it up to date. (Also, annoying that test-env-log broke the build with a semver-compatible update, but 🤷)

@jplatte
Copy link
Member Author

jplatte commented Nov 23, 2021

(Also, annoying that test-env-log broke the build with a semver-compatible update, but 🤷)

It only broke CI due to -D warnings.

@jneem jneem merged commit 6e495ad into linebender:master Nov 26, 2021
@jplatte jplatte deleted the tracing-bump branch November 26, 2021 15:09
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.

5 participants