Skip to content
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

shared/cmd: Allow a cmd asker to be created with a logger instance #13859

Merged
merged 2 commits into from
Aug 27, 2024

Conversation

gabrielmougard
Copy link
Contributor

@gabrielmougard gabrielmougard commented Aug 2, 2024

If a command asker is with a logger, the question and answer (even if it is invalid) are systematically logged to the logger destination. This will greatly help debugability in integration test suites like in MicroCloud which have a lot of interactions in the command line.

masnax
masnax previously approved these changes Aug 2, 2024
Copy link
Contributor

@masnax masnax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds reasonable to me, though one nit I have is that verbose is kind of ambiguous for a feature like this which is intended for debugging. Not necessarily a blocker.

Copy link
Member

@tomponline tomponline left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed the other day, lets change this to allow passing in a logger for verbose logging rather than outputting to stdout.

If a command asker is set with a logger instance, the question and answer (even if it is invalid)
are systematically logged. This will greatly help debugability in integration test suites
like in MicroCloud which have a lot of interactions in the command line.

Signed-off-by: Gabriel Mougard <[email protected]>
@gabrielmougard gabrielmougard changed the title shared/cmd: Allow a cmd asker to be set in 'verbose' mode shared/cmd: Allow a cmd asker to be created with a logger instance Aug 23, 2024
@gabrielmougard
Copy link
Contributor Author

@tomponline updated

@tomponline
Copy link
Member

@gabrielmougard linting issues need fixing

@gabrielmougard gabrielmougard force-pushed the feat/asker-in-debug-mode branch 2 times, most recently from 40d7144 to 80e2400 Compare August 23, 2024 12:45
@tomponline tomponline merged commit a894583 into canonical:main Aug 27, 2024
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants