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

Add the ability to prune non-error entries out of the graph #218

Merged
merged 2 commits into from
Nov 6, 2018

Conversation

r-hang
Copy link
Contributor

@r-hang r-hang commented Oct 26, 2018

No description provided.

@CLAassistant
Copy link

CLAassistant commented Oct 26, 2018

CLA assistant check
All committers have signed the CLA.

@r-hang r-hang force-pushed the rhang/prune-grafx-graph branch 2 times, most recently from 0b2091b to 12eb650 Compare October 26, 2018 01:51
@r-hang r-hang changed the title Rhang/prune grafx graph Add the ability to prune non-error entries out of the graph Oct 26, 2018
internal/dot/graph.go Outdated Show resolved Hide resolved
internal/dot/graph.go Outdated Show resolved Hide resolved
internal/dot/graph.go Outdated Show resolved Hide resolved
@r-hang r-hang force-pushed the rhang/prune-grafx-graph branch 2 times, most recently from a6f44ff to eb47e5e Compare October 29, 2018 16:42
@codecov
Copy link

codecov bot commented Oct 29, 2018

Codecov Report

Merging #218 into master will decrease coverage by 0.29%.
The diff coverage is 95%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #218     +/-   ##
=========================================
- Coverage   98.93%   98.64%   -0.3%     
=========================================
  Files           9        9             
  Lines        1032     1104     +72     
=========================================
+ Hits         1021     1089     +68     
- Misses          9       11      +2     
- Partials        2        4      +2
Impacted Files Coverage Δ
dig.go 98.73% <100%> (ø) ⬆️
internal/dot/graph.go 97.81% <94.93%> (-2.19%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cc30ae7...c23bc0c. Read the comment docs.

@r-hang r-hang force-pushed the rhang/prune-grafx-graph branch 2 times, most recently from be8cb66 to bc3c7e0 Compare October 30, 2018 21:37
dig.go Outdated Show resolved Hide resolved
internal/dot/graph.go Outdated Show resolved Hide resolved
internal/dot/graph.go Outdated Show resolved Hide resolved
internal/dot/graph.go Outdated Show resolved Hide resolved
internal/dot/graph.go Outdated Show resolved Hide resolved
internal/dot/graph.go Outdated Show resolved Hide resolved
internal/dot/graph.go Outdated Show resolved Hide resolved
internal/dot/graph.go Outdated Show resolved Hide resolved
internal/dot/graph.go Outdated Show resolved Hide resolved
internal/dot/graph_test.go Outdated Show resolved Hide resolved
@r-hang r-hang force-pushed the rhang/prune-grafx-graph branch 5 times, most recently from b2aad24 to 9faa55f Compare October 31, 2018 17:22
Copy link
Collaborator

@abhinav abhinav left a comment

Choose a reason for hiding this comment

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

Offline discussion, maybe something like this:

type ResultConsumer interface {
    // Asks the consumer to delete the reference to this Result.
    deleteResult(*Result)
}

// Ctor and Group implement ResultConsumer.

type Result struct {
    // ...
    
    Consumers []ResultConsumer
}

internal/dot/graph.go Show resolved Hide resolved
internal/dot/graph.go Outdated Show resolved Hide resolved
internal/dot/graph.go Show resolved Hide resolved
internal/dot/graph.go Outdated Show resolved Hide resolved
internal/dot/graph.go Outdated Show resolved Hide resolved
internal/dot/graph.go Outdated Show resolved Hide resolved
internal/dot/graph.go Outdated Show resolved Hide resolved
internal/dot/graph_test.go Outdated Show resolved Hide resolved
internal/dot/graph_test.go Outdated Show resolved Hide resolved
internal/dot/graph_test.go Outdated Show resolved Hide resolved
internal/dot/graph_test.go Outdated Show resolved Hide resolved
internal/dot/graph_test.go Outdated Show resolved Hide resolved
@r-hang r-hang force-pushed the rhang/prune-grafx-graph branch 6 times, most recently from 4332f45 to a0984fc Compare November 3, 2018 01:52
internal/dot/README.md Outdated Show resolved Hide resolved
internal/dot/README.md Show resolved Hide resolved
internal/dot/graph.go Outdated Show resolved Hide resolved
internal/dot/graph.go Show resolved Hide resolved
Added a Prune method to remove entries from the graph that do not
contain any failing results. Graph nodes that are not root or
transitive failures do not help the user debug and make it
more difficult to debug by cluttering the graph.

Orient the graph left to right for graph readability
@r-hang r-hang merged commit 3ff03ed into uber-go:master Nov 6, 2018
@r-hang r-hang deleted the rhang/prune-grafx-graph branch November 6, 2018 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants