Skip to content

Conversation

@mfukar
Copy link
Contributor

@mfukar mfukar commented Aug 28, 2025

Add some basic descriptions for the headers in qdlt/ so that some relationships are clear in generated graphs.

This is not complete by far, most functions miss documentation and there are no statements of intent anywhere.

Copy link
Collaborator

@vifactor vifactor left a comment

Choose a reason for hiding this comment

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

This is a good analysis but comments looks often like a notes for yourself. If you intend to merge the MR, please keep only comments where you know what function/class does. If role of a code is not clear, just don't write a comment.

@mfukar
Copy link
Contributor Author

mfukar commented Aug 29, 2025

This is a good analysis but comments looks often like a notes for yourself. If you intend to merge the MR, please keep only comments where you know what function/class does. If role of a code is not clear, just don't write a comment.

I can keep working on this change to find out concrete information and have it reflected in the comments you are referring to, and I can also remove them instead, but I think it would be preferable by everyone if instead those notes were replaced by actual facts. Most of them have to do with intent which I am not aware of and I would prefer if the team helped me understand those pieces - and improve the PR accordingly.

@mfukar mfukar marked this pull request as draft September 2, 2025 09:38
@mfukar mfukar force-pushed the qdlt-docx-1 branch 2 times, most recently from 5046263 to 5a1dbf9 Compare September 3, 2025 13:38
@mfukar mfukar marked this pull request as ready for review September 8, 2025 08:02
* Add some basic descriptions for the headers in qdlt/
  so that some relationships are clear in generated graphs.
* Some basic docs in locations where code is parsing DLT message(s)
  or DLT messages are created based on internal state.

This is not complete by far, most functions miss documentation
and there are no statements of intent anywhere.

Signed-off-by: Michael Foukarakis <[email protected]>
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.

2 participants