laze icon indicating copy to clipboard operation
laze copied to clipboard

feat!: Use env_logger for output

Open bergzand opened this issue 2 months ago • 8 comments

This refactors all (non-commented) println!s to use a logger via env_logger. This allows for setting the log level via environment variables and retains the filtering of the messages via the command line arguments (--verbose and --quiet).

A lot of the code is simplified because the verbose argument doesn't have to be passed around in most cases.

bergzand avatar Oct 20 '25 10:10 bergzand

Needs #799 for the CI, and I have to take a look where the nightly test is failing exactly

bergzand avatar Oct 20 '25 10:10 bergzand

Needs #799 for the CI, and I have to take a look where the nightly test is failing exactly

#799 is optional, though it would be nice to briefly check if there's any performance impact.

kaspar030 avatar Oct 20 '25 11:10 kaspar030

A number of println!s have a "laze:" at the start of the output. Do you want to retain that, and should that be in the main log write! template instead?

bergzand avatar Oct 20 '25 11:10 bergzand

A number of println!s have a "laze:" at the start of the output. Do you want to retain that, and should that be in the main log write! template instead?

Well, maybe keep changing output method and changing output content separately.

I did make laze output laze: ... because it might intermingle with e.g., ninja's output. I don't think all of laze's output prefixes laze:, so not sure it can go into the main log template. Would be nicer. Is there opt-out?

kaspar030 avatar Oct 20 '25 11:10 kaspar030

I did make laze output laze: ... because it might intermingle with e.g., ninja's output.

That's what I assumed the reason was for the prefix. I do think it should be applied consistently, otherwise messages that do not have the prefix might confuse the reader of the origin.

Would be nicer. Is there opt-out?

An opt out of the prefix? Should be possible to add in a follow up.

Well, maybe keep changing output method and changing output content separately.

I already removed a 'warn' in the output content here and there, because that's duplicate now.

bergzand avatar Oct 20 '25 11:10 bergzand

Output should now be identical to current main.

bergzand avatar Oct 21 '25 08:10 bergzand

🐰 Bencher Report

Branchpr/logger
Testbedgithub-actions
Click to view all benchmark results
Benchmarkperf:task-clockBenchmark Result
msec x 1e3
(Result Δ%)
Upper Boundary
msec x 1e3
(Limit %)
laze -C RIOT build --global --generate-only📈 view plot
🚷 view threshold
9.34 x 1e3
(-1.07%)Baseline: 9.44 x 1e3
9.94 x 1e3
(94.01%)
🐰 View full continuous benchmarking report in Bencher

github-actions[bot] avatar Oct 21 '25 08:10 github-actions[bot]

Coverage Status

coverage: 80.796% (+0.6%) from 80.182% when pulling bfd1ad4e5297a7fbf20bd4b084f524d8f356f3ac on bergzand:pr/logger into f1a88ed7de7b6bed8a6ffb4324518197791e3b04 on kaspar030:main.

coveralls avatar Oct 21 '25 08:10 coveralls