rt icon indicating copy to clipboard operation
rt copied to clipboard

Graphviz link traversal rewrite to breadth-first

Open atus42 opened this issue 9 years ago • 9 comments

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.

atus42 avatar Sep 05 '14 11:09 atus42

Is this problem really so marginal that no one watched / commented it?

atus42 avatar Jun 29 '15 08:06 atus42

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.

sartak avatar Jun 29 '15 15:06 sartak

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.

atus42 avatar Jun 30 '15 07:06 atus42

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.

sartak avatar Jul 01 '15 17:07 sartak

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?

atus42 avatar Jul 03 '15 06:07 atus42

still can not see it is merged in trunk-4.2

atus42 avatar Aug 12 '15 13:08 atus42

Hi atus42, it's still on my list to review. Sorry about the confusion.

sartak avatar Aug 12 '15 15:08 sartak

I understand the delay now. Thanks for clearing.

atus42 avatar Aug 13 '15 06:08 atus42

Almost two months past. Can I help in the review?

atus42 avatar Oct 08 '15 07:10 atus42