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

feat(graph): also generate the transitive reduction #2282

Merged
merged 4 commits into from May 10, 2024

Conversation

erights
Copy link
Contributor

@erights erights commented May 10, 2024

Closes: #XXXX
Refs: Agoric/agoric-sdk#9359

Description

Use the "tred" executable that seems to already be bundled with graphviz to

# Also generates visualizations of the transitive reduction (tred) of
# that graph, which is the minimal graph with the same *transitive*
# dependencies. Much more legible by itelf. Seeing the two side by side
# often helps to understand the full picture.

At the time of writing, the current graph of endo dependencies is

packages-graph

and the transitive reduction of those is

packages-graph-tred

Notice how the columns mostly align. Unfortunately, the columns don't fully align, and the vertical order of elements within each column is not the same. For viewing them side by side for better understanding, it would be better if both of these annoyances were fixed. But I have no idea how to do that, so this PR does not try.

Security Considerations

none

Scaling Considerations

none

Documentation Considerations

potentially helps someone who wants to understand how our system is internally layered, which is the point. Otherwise, none

Testing Considerations

none

Compatibility Considerations

none

Upgrade Considerations

none

@erights erights self-assigned this May 10, 2024
@erights erights marked this pull request as ready for review May 10, 2024 21:29
@erights erights requested review from turadg and kriskowal May 10, 2024 21:29
Copy link
Member

@turadg turadg left a comment

Choose a reason for hiding this comment

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

helpful!

@erights erights force-pushed the markm-graph-transitive-reduction branch from e63138e to cf215d0 Compare May 10, 2024 23:32
@erights erights enabled auto-merge (squash) May 10, 2024 23:32
@erights erights merged commit b02f0e2 into master May 10, 2024
17 checks passed
@erights erights deleted the markm-graph-transitive-reduction branch May 10, 2024 23:39
mergify bot pushed a commit to Agoric/agoric-sdk that referenced this pull request May 11, 2024
closes: #XXXX
refs: endojs/endo#2282

## Description

Use the "tred" executable that seems to already be bundled with graphviz
to

```
# Also generates visualizations of the transitive reduction (tred) of
# that graph, which is the minimal graph with the same *transitive*
# dependencies. Much more legible by itelf. Seeing the two side by side
# often helps to understand the full picture.
```

At the time of writing, the current graph of agoric-sdk dependencies is


![packages-graph](https://github.com/Agoric/agoric-sdk/assets/273868/4fbcba59-6081-41fd-8866-7372b14a7696)

and the transitive reduction of those is


![packages-graph-tred](https://github.com/Agoric/agoric-sdk/assets/273868/ef7e4b4e-986c-4bc6-96cc-df8587272fd5)

Notice how the columns *mostly* align. Unfortunately, the columns don't
fully align, and the vertical order of elements within each column is
not the same. For viewing them side by side for better understanding, it
would be better if both of these annoyances were fixed. But I have no
idea how to do that, so this PR does not try.

### Security Considerations

none
### Scaling Considerations

none
### Documentation Considerations

potentially helps someone who wants to understand how our system is
internally layered, which is the point. Otherwise, none
### Testing Considerations

none
### Compatibility Considerations

none
### Upgrade Considerations

none
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.

None yet

2 participants