rami3l

Results 467 comments of rami3l

I just checked and it looks like tokio-console doesn’t currently have a timeline view (https://github.com/tokio-rs/console/issues/129), so I imagine `opentelemetry` and `jaeger` are here to stay for longer... For now I...

The CI now works, but I have no idea how to properly inject the `otel` subscriber into tests, because it's a global state, but the new `stderr` subscriber is local...

Regarding https://github.com/rust-lang/rustup/pull/3803#issuecomment-2143703392, I think a reasonable solution would be making all unit tests run under `#[tokio::test]`, so that we can stick to a new local subscriber all the time. This...

@djc The new approach proposed in https://github.com/rust-lang/rustup/pull/3871#pullrequestreview-2115593544 has worked! This PR has been thus unblocked from test refactoring and I'll just need to do some rather simple tweaks (see https://github.com/rust-lang/rustup/pull/3803#issuecomment-2092349333...

@rbtcollins @djc I've just finished all 3 cleanup tasks, unfortunately however the 3rd (the final) commit (https://github.com/rust-lang/rustup/pull/3803/commits/0cf0c6bdd35f9bf8076d78627eab0ee6f01e3a88) has made our CI red. To make sure that only this commit was...

> Maybe try a branch where you apply [0cf0c6b](https://github.com/rust-lang/rustup/commit/0cf0c6bdd35f9bf8076d78627eab0ee6f01e3a88) to 1.27.1 directly (or some other commit maybe from before #3868) to see if that works? It might be some "interesting"...

> Seemingly this has been fixed on main. I have added a regression test here: https://github.com/rust-lang/rustup/pull/4040. @lucacasonato Thanks for the heads up! This looks interesting. Maybe that's a side effect...

> > > Seemingly this has been fixed on main. I have added a regression test here: [#4040](https://github.com/rust-lang/rustup/pull/4040). > > > > > > [@lucacasonato](https://github.com/lucacasonato) Thanks for the heads up!...

> Given that `Cfg::get_profile()` should yield the default profile anyway, it shouldn't be a substantial change? It does feel more correct to initialize with `None` than `Some(Profile::default())`. @djc You're right....

@apps4uco Thanks for filing this issue! This is surely a thing to be considered.