search

discourse / prometheus_exporter

A framework for collecting and aggregating prometheus metrics
MIT License
525 stars 153 forks source link

Various fixes to Sidekiq classes #193

Closed mrsimo closed 2 years ago

mrsimo commented 2 years ago

Hello! I found these little things while thinking about another possible contribution I might have.

The first two are mistakes I made, one using the wrong class, and the other must have been copying the test when adding the all_queues option... and there were two tests with the same name. I missed the warnings, as I also use WSL, but with your change to the test server in https://github.com/discourse/prometheus_exporter/commit/cbe1d51e2a0888a1801fc1f248e144cb4872a2fb it was fixed 🎉 and saw these warnings.

There's a cosmetic commit that's purely preference, but these tests felt very verbose already and it seemed like the things I changed would be more readable as single lines. Perfectly fine removing this commit from the PR if you don't like it.

The last commit is another thing I caught once I saw the warnings.

Thank you!

SamSaffron commented 2 years ago

I see, looks good to me, thanks @mrsimo