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

Tracer documentation generation enhacements #5504

Merged
merged 7 commits into from
Oct 13, 2023
Merged

Conversation

jutaro
Copy link
Contributor

@jutaro jutaro commented Oct 11, 2023

This pull request addresses several enhancements related to Tracer documentation generation, ensuring consistency and improving the quality of the documentation. The key changes in this pull request include:

Addition of Main Part of Consistency Checker to Trace-Dispatcher: We've introduced the main part of the consistency checker to the trace-dispatcher, which ensures that traces remain consistent and accurate.

Enhancements to Documentation Generation: We've made improvements to the documentation generation process, resulting in more comprehensive and user-friendly documentation for the Tracer system.

Consistent Renaming for TraceControl Members: To maintain clarity and consistency within the codebase, we've applied consistent renaming to TraceControl members.

Documentation for Reflection Messages: We've added documentation for reflection messages, making it easier for developers to understand the purpose and usage of these messages.

@jutaro jutaro self-assigned this Oct 11, 2023
* Add main part of consistency checker to trace-dispatcher
* Enhance docu generation 
* Consistent renaming renaming for TraceControl members
* Reflection messages documentation
* chaindb namespace fix
* Move main part of consistency check to trace-dispatcher
* Missing parts for doc generation
and fix typos
@jutaro jutaro force-pushed the jutaro/tracer-documentation branch from e3b396e to bb93888 Compare October 12, 2023 14:03
@jutaro jutaro marked this pull request as ready for review October 12, 2023 14:05
trace-dispatcher/src/Cardano/Logging/DocuGenerator.hs Outdated Show resolved Hide resolved
import Cardano.Logging.ConfigurationParser
import Cardano.Logging.Types

-- | Warniings as a list of text
Copy link
Contributor

Choose a reason for hiding this comment

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

typo in comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok


-- | Check if a single namespace is legal. Returns just a warning test,
-- if this is not the case
checkNamespace :: NSLookup -> [T.Text] -> Maybe T.Text
Copy link
Contributor

@mgmeier mgmeier Oct 13, 2023

Choose a reason for hiding this comment

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

Please could you add a detailed comment of what 'legal' / consistent / well-formed actually means?
From the code I can infer that a namespace must have a valid lookup at each level of nesting to be considered legal. But what abstract condition is checked by that?

Maybe make a joint [Note] including comment from asNSLookup - it's not straightforward to see how the nested Map values are constructed from [[Text]]. But this is vital to understand how the check works. You could illustrate by including an example?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

numbers = fromString $ show (length dtBuilderList) <> " log messages." <> "\n\n"
legend = fromString $ "\9443 - This is the root of a tracer\n\n"
<> "\9442 - This is the root of a tracer that is silent because of the current configuration\n\n"
<> "\9436 - This is the root of a tracer, that provides metrics\n\n"
Copy link
Contributor

@mgmeier mgmeier Oct 13, 2023

Choose a reason for hiding this comment

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

A general suggestion for these special characters: can we instead of inlining them, use a top-level definition (and use hex encoding for UTF codes), like so:

utf16CircledT :: Builder
utf16CircledT = singleton '\x24E3'

This would make any maintenance / change to the special chars much more easy. (EDIT: the type was just an example, it could as well be a Text)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

@mgmeier mgmeier left a comment

Choose a reason for hiding this comment

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

Thank you @jutaro LGTM

@jutaro jutaro enabled auto-merge October 13, 2023 17:00
@jutaro jutaro added this pull request to the merge queue Oct 13, 2023
Merged via the queue into master with commit b7edd2f Oct 13, 2023
21 checks passed
@jutaro jutaro deleted the jutaro/tracer-documentation branch October 13, 2023 18:41
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.

None yet

2 participants