-
Notifications
You must be signed in to change notification settings - Fork 7
chore: improve pretty collector format #107
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Change SummaryThis PR refactors the Issues FoundI've identified 3 issues:
The refactoring successfully achieves its goals of improved formatting and testability. The test coverage is comprehensive with both boot-time and Unix timestamp scenarios. |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Signed-off-by: Wim Looman <[email protected]>
Signed-off-by: Wim Looman <[email protected]>
Signed-off-by: Wim Looman <[email protected]>
f127b98 to
e068544
Compare
|
/claude-review |
Change SummaryThis PR refactors the All previously identified issues have been addressed in the latest commits. ✅ Review complete - no new issues found |
| use std::vec::Vec; | ||
|
|
||
| #[test] | ||
| fn smoke_test() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I personally prefer naming this test by what it tests, but is not a blocker. Feel free to resolve.
c72f48e to
ff0c090
Compare
* Don't show any trailing text if there are no attributes * Use [] instead of "" to delimit the attributes * Remove leading `, ` before the first attribute * Debug print strings to tell where they end Signed-off-by: Wim Looman <[email protected]> Co-authored-by: arctic-alpaca <[email protected]>
ff0c090 to
bf9f4e6
Compare
* Refactor of the pretty-printer to make it testable. * Improve formatted output. * Print to stderr instead of stdout. --------- Signed-off-by: Wim Looman <[email protected]> Co-authored-by: arctic-alpaca <[email protected]>
* Refactor of the pretty-printer to make it testable. * Improve formatted output. * Print to stderr instead of stdout. --------- Signed-off-by: Wim Looman <[email protected]> Co-authored-by: arctic-alpaca <[email protected]>
From #107 (comment). Signed-off-by: Wim Looman <[email protected]>
I ended up doing a bit more refactoring of the pretty-printer to get it both testable and a format that I think is readable enough to merge. With the test you can step through the commits and see how the format changes with each.
I've targeted this for @arctic-alpaca's branch and propose we merge into that first, then merge it together as a whole.