search

tokio-rs / tracing

Application level tracing for Rust.
https://tracing.rs
MIT License
5.15k stars 673 forks source link

Remove the matchers crate #2945

Open wyatt-herkamp opened 2 months ago

wyatt-herkamp commented 2 months ago

Motivation

The matcher's crate has not been updated in 3 years. It is on Regex Automata version "0.1". Due to this I am getting a weird utf-8 error. This part is not a tracing problem. Just an outdated library issue.

Solution

This removes the matchers crate. The code is inspired from matchers crate with updates to Regex Automata 0.4.6. This also strips down the code as we don't need all the features matchers provided.

All tests were passed and has zero user facing changes.

Also this adds .vscode/settings.json to the .gitignore as that file can be developer settings. For instance I needed to enable all features for rust-analyzer.

wyatt-herkamp commented 2 months ago

Funny story. The file was corrupted on my system so that bug I was seeing was a corrupted local cargo registry. However, I still think it is a good idea to remove the matchers crate as it seems to be no longer maintained.

wyatt-herkamp commented 2 months ago

There seems to still be at least some development there, the last comment about the update to a more recent reflex automata version is from March.

I did see people are trying to update the crate. However, the developer doesn't seem to be making it a high priority. With it only being about 100 lines of code to implement I don't think using the matchers crate is providing a huge benefit here.

The matchers code feels a bit out of places in an ideal world we could just update the code there.

I understand. However, we are not using all the features that the matchers crate provides. And shrinking the dependency tree can be a good thing as it can lead to better compile times and less likely to get attacked via a supply chain attack.

sophie-h commented 2 days ago

matchers 0.2 with updated dependency is released now. It would be great if this issue could be resolved one or the other way.

wyatt-herkamp commented 23 hours ago

Should I go ahead and close this PR then?