rt
rt copied to clipboard
Graphviz link traversal rewrite to breadth-first
Graphviz link traversal finds tickets by child-first algorithm now. This can lead to miscalculate ticket depth and eventually tickets left out. Displayed ticket properties can differ from the settings due to miscalculated depth.
Let's say you have ticket A as the 'root' or top of the tree. It has a child B and C. B also has C as a child. In this scenario A have level 0, B have level 1 and C have level 2 because it was found first as the child of B. When it is found again as child of A, it is skipped because it is recorded in the 'seen' table.
So if I want to see all first level child of A, C will be left out. If I display second level child too, it will have the properties displayed as a second level child however it is rendered properly to the first line as a first level child.
Is this problem really so marginal that no one watched / commented it?
Hi atus42. Thanks for your contribution. Could you explain in your commit message why this change is being made? Why is breadth-first search better? The existing comment of "Howewer the patch I wrote to this module solves the problem of skipping links, ticket depth is still miscalculated. Now I made the full rewrite and make it process the tickets level by level." belongs in a comment in the pull request, not in the commit message. The commit message must stand on its own and should justify why the change exists at all.
Unfortunately I didn't find the virtual machine where I made this commit so I was not able to change the commit message, but rewrite the first comment to explain why the change is necessary.
It is a continuation/finalization of the pull request #103 made by me.
I understand the context now. Thanks very much!
I'll reword the commit message as I cherry-pick it into 4.2-trunk, so don't worry about that.
In the history of my other pull requests I saw a 'merged commit' message when the actual merge happened but I can not see one here. Did you forget the commit or it happened a way that is not visible here?
still can not see it is merged in trunk-4.2
Hi atus42, it's still on my list to review. Sorry about the confusion.
I understand the delay now. Thanks for clearing.
Almost two months past. Can I help in the review?