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

Improve private/buf/dag #2240

Merged
merged 3 commits into from
Jun 29, 2023
Merged

Improve private/buf/dag #2240

merged 3 commits into from
Jun 29, 2023

Conversation

bufdev
Copy link
Member

@bufdev bufdev commented Jun 29, 2023

  • Add NumNodes to return the number of nodes.
  • Add NumEdges to return the number of edges.
  • Rename Walk to WalkEdges
  • Fix a bug where WalkEdges incorrectly returned an error when there were only nodes in the graph with no edges.
  • Add WalkNodes that walks the nodes. This allows access to nodes with no edges.
  • Update DOTString to print nodes without edges.
  • Add testing.

@bufdev bufdev requested a review from unmultimedio June 29, 2023 17:34
@doriable doriable merged commit 82a1bbc into main Jun 29, 2023
@doriable doriable deleted the dag-additions branch June 29, 2023 18:46
@@ -195,6 +241,34 @@ func (g *Graph[Key]) DOTString(keyToString func(Key) string) (string, error) {
); err != nil {
return "", err
}
// We also want to pick up any nodes that do not have edges, and display them.
if err := g.WalkNodes(
func(key Key, inboundEdges []Key, outboundEdges []Key) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
func(key Key, inboundEdges []Key, outboundEdges []Key) error {
func(key Key, inboundEdges, outboundEdges []Key) error {

// We also want to pick up any nodes that do not have edges, and display them.
if err := g.WalkNodes(
func(key Key, inboundEdges []Key, outboundEdges []Key) error {
//
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
//

@@ -180,9 +180,9 @@ func TestWalk2(t *testing.T) {
)
}

func TestWalk3(t *testing.T) {
func TestWalkEdges3(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: test names should be more descriptive instead of Test<Num>

Comment on lines +137 to +141
// f is called for each directed edge. The first argument is the source
// node, the second is the destination node.
//
// Returns a *CycleError if there is a cycle in the graph.
func (g *Graph[Key]) Walk(f func(Key, Key) error) error {
func (g *Graph[Key]) WalkEdges(f func(Key, Key) error) error {
Copy link
Member

Choose a reason for hiding this comment

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

This could be in the func signature instead of the doc

Suggested change
// f is called for each directed edge. The first argument is the source
// node, the second is the destination node.
//
// Returns a *CycleError if there is a cycle in the graph.
func (g *Graph[Key]) Walk(f func(Key, Key) error) error {
func (g *Graph[Key]) WalkEdges(f func(Key, Key) error) error {
// f is called for each directed edge.
//
// Returns a *CycleError if there is a cycle in the graph.
func (g *Graph[Key]) WalkEdges(f func(sourceNode Key, destinationNode Key) error) error {

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.

4 participants