search

loov / goda

Go Dependency Analysis toolkit
MIT License
1.37k stars 45 forks source link

graph: repository and module clustering #60

Closed ChrisHines closed 2 years ago

ChrisHines commented 2 years ago

I don't find the clusters chosen by goda -cluster map to concepts in Go. I think clustering based on repository and module membership provides a more useful visualization. Here's a rough draft of some changes to make goda -cluster work that way. Is this something you would consider adding to goda if it was polished up a bit more?

egonelbre commented 2 years ago

In principle, yes. I'll take a deeper look into this PR in a bit...

The reason the "cluster" thing feels slightly awkward is that I don't have a good model how to do all the things I want to do. For example, https://pkg.go.dev/golang.org/x/tools is a single module, however probably shouldn't be represented as a single subgraph. Of course, the current hierarchical approach also doesn't fit nicely either.

Similarly, I wanted to have some way of collapsing and un-collapsing the graph... https://github.com/loov/goda/issues/41 -- however, that shouldn't be too much additional complexity on the top. e.g. what happens when you collapse multiple modules in to a single "item". The main reason for collapsing are things like internal/*, which don't add much useful things to the graph. This means that it would need a way to represent "module", "package" and "group" in the same graph and links between them.

And final issues is that is "versions". e.g. what if you want query a delta between different versions of packages. Even now there's a slight version drift problem, when you query go list A:all - B:all then the subpackage versions in A and B might be different.

ChrisHines commented 2 years ago

Yes. I understand that you have more ambitious plans, and they sound nice and powerful. I didn't want to tackle all of that at once, but I did want a way to visualize repository and module boundaries and have module versions displayed on the graph as well. So that's what this set of changes does. I can use my fork for my immediate needs, but I wanted to share what I did with you in case it was useful.

It seems to me that implementing your full ambitions will require additions to the query language or maybe adding a "layout language" to control how the packages in the queried package-set map to the output. That was more than I wanted to think about while I was still learning the goda code base.

I'm happy to provide feedback or help brainstorm ideas if you want.

egonelbre commented 2 years ago

Ah, I definitely didn't mean that the plans should happen in this PR. As long as something is an improvement, then I'm all for it.

I'm giving more context to the problem, in case you have some nice ideas :). So yeah, those topics are more of an over-arching goal and I don't want to block things that are immediately useful. Module based grouping would've been part of the final solution as well.

ChrisHines commented 2 years ago

As expected moving pkggraph.Tree into its own package clarified the names and meant we could avoid renaming pkggraph.Node. I've made this ready for review. Let me know if you see anything else to improve.

egonelbre commented 2 years ago

Looks good to me. Thanks.