search

estk / log4rs

A highly configurable logging framework for Rust
Apache License 2.0
973 stars 143 forks source link

apply no_color w/ windows cfg #335

Closed bconn98 closed 6 months ago

bconn98 commented 7 months ago

Test MR to add windows support to #302

codecov-commenter commented 7 months ago

Codecov Report

Attention: 17 lines in your changes are missing coverage. Please review.

Comparison is base (58b92c8) 61.12% compared to head (66c6e91) 60.81%.

Files Patch % Lines
src/encode/writer/console.rs 39.28% 17 Missing :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #335 +/- ## ========================================== - Coverage 61.12% 60.81% -0.32% ========================================== Files 23 23 Lines 1407 1429 +22 ========================================== + Hits 860 869 +9 - Misses 547 560 +13 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

bconn98 commented 7 months ago

@estk It seems to me that the runners have different color controls already enabled that conflict with our tests. Locally the test results look good to me. In the CI only CLICOLOR_FORCE works and only for Ubuntu. I think that's acceptable, we will just need to verify those tests locally when there are related changes. @lelongg you should be able to grab my MR for the test updates and windows portion

lelongg commented 6 months ago

Is #302 still needed if this one is merged ?

bconn98 commented 6 months ago

Up to you if you want to just pull my changes into your MR. I just wanted to help out with the windows/test side of things while I had the time. On Jan 5, 2024, at 09:36, Gérald Lelong @.***> wrote: Is #302 still needed if this one is merged ?

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: @.***>

bconn98 commented 6 months ago

@lelongg FYI I just got my windows machine back up and running and can confirm that the changes made work for windows as well. I'll leave it up to you if you would like me to make this MR against #302 and roll it all up under your MR or just push this one. 😄