-
Notifications
You must be signed in to change notification settings - Fork 337
Add message module for terminal output #263
Conversation
looks like you have an import error and some dead code warnings! lemme know if you need help with those :) |
adding @granjef3 as a reviewer as we'll tap him to finish out this migration :) |
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 like this cleanup a lot. The unused emojis/functions clippy sees are definitely useful, and I think it would be better to find a place for them to be used then to delete them.
Co-Authored-By: Matt <[email protected]>
dc85963
to
07e9767
Compare
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.
lgtm! there's a little more work left to audit the whole codebase and make sure everything is consistent- i'd recommend merging this ASAP to establish the pattern, maybe @granjef3 can take on the full audit post merge?
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.
LGTM. quick grep for stray "println!" only shows some in the installer.
fixes #219