Create the Logging and Output guidelines#4935
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds a new 'Logging and Output' section to the contribution documentation (docs/contribute.rst) to clarify the distinction between logging (stderr) and output (stdout), and to define verbosity and debug levels. Feedback recommends formatting the logging method names as inline literals for consistent rendering, and adding a note to clarify that using the level parameter with info() is a planned target state, as the current implementation requires using verbose() instead.
| * ``info()`` / level 0 — key output: plan names, step names, | ||
| phase summary, overall summaries | ||
| * ``info(level=1)`` — specific info: individual test names in | ||
| ``discover`` and ``execute`` |
There was a problem hiding this comment.
The separation between these 2 is still vague in the wording. I don't feel like I can answer yet when a potentially loggable output should be:
- info0
- info1
- moved to debug
- not logged at all
Let me give a concrete snippet
tmt/tmt/steps/prepare/artifact/providers/repository.py
Lines 82 to 83 in bae12b9
There was a problem hiding this comment.
The two levels are actually described using the current tmt run behavior. Overal summaries versus listing individual tests. If you have more tangible examples at hand, please suggest them.
There was a problem hiding this comment.
If you have more tangible examples at hand, please suggest them.
What is wrong with the example snippet given above?
There was a problem hiding this comment.
I mean, example of how to better write the summary, for those two levels.
There was a problem hiding this comment.
Well, first is the question of what to do in that example case. Because I do not know how it should be handled and as such I don't know what info0 and info1 should have
There was a problem hiding this comment.
After reading the level summaries...
self.logger.info(f"Reading repository file from local path: {parsed_path}")
seems like this one should be log level 1
self.logger.debug(f"Absolute path of the repository file: '{parsed_path.resolve()}'")
This one should be debug level 2
If those choices seem wrong, then maybe the summary for those two levels could be changed 😆
6f280e7 to
5c2880e
Compare
Add a new section defining the individual output types and individual verbosity levels. Describes the target state to which we want to get.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: Cristian Le <github@lecris.me>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
e2aa8e1 to
c31c597
Compare
| method. Output represents the actual data result of a command — | ||
| a list of names, exported metadata, or formatted details meant |
There was a problem hiding this comment.
| method. Output represents the actual data result of a command — | |
| a list of names, exported metadata, or formatted details meant | |
| method. Output represents the actual data result of a command, such as | |
| a list of names, exported metadata, or formatted details meant |
| execution. It is the primary form of communication for commands | ||
| that do not produce structured data, such as: | ||
|
|
||
| * ``tmt run`` | ||
| * ``tmt try`` | ||
| * ``tmt lint`` | ||
| * ``tmt clean`` |
There was a problem hiding this comment.
| execution. It is the primary form of communication for commands | |
| that do not produce structured data, such as: | |
| * ``tmt run`` | |
| * ``tmt try`` | |
| * ``tmt lint`` | |
| * ``tmt clean`` | |
| execution. This is the main mode of communication you should use throughout the code. |
Main thing here is to:
- mention that this is the main thing to focus on
- the developer should not have to figure out how one part of the code is used in either
tmt runortmt ls
| * ``info()`` / level 0 — key output: plan names, step names, | ||
| phase summary, overall summaries | ||
| * ``info(level=1)`` — specific info: individual test names in | ||
| ``discover`` and ``execute`` |
There was a problem hiding this comment.
Well, first is the question of what to do in that example case. Because I do not know how it should be handled and as such I don't know what info0 and info1 should have
| when the topic is explicitly enabled. The following topics are | ||
| available: |
There was a problem hiding this comment.
Would leave out the enumeration of topics. It's a recipe for divergence. Instead we can add docstrings to log.Topic if needed.
There was a problem hiding this comment.
If these are moved to log.Topic as docstrings, having a link to the docstrings here would be very helpful
Add a new section defining the individual output types and individual verbosity levels. Describes the target state to which we want to get.
Fix #4814.
Pull Request Checklist