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 #9359

Merged
merged 4 commits into from
May 11, 2024

Conversation

erights
Copy link
Member

@erights erights commented May 10, 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

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

Copy link

cloudflare-pages bot commented May 10, 2024

Deploying agoric-sdk with  Cloudflare Pages  Cloudflare Pages

Latest commit: be3b9b8
Status: ✅  Deploy successful!
Preview URL: https://f067d2df.agoric-sdk.pages.dev
Branch Preview URL: https://markm-graph-transitive-reduc.agoric-sdk.pages.dev

View logs

@erights erights marked this pull request as ready for review May 10, 2024 21:41
@erights erights requested review from turadg and kriskowal May 10, 2024 21:46
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!

Be sure to automerge:squash

@erights erights added the automerge:squash Automatically squash merge label May 10, 2024
erights added a commit to endojs/endo that referenced this pull request 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](https://github.com/endojs/endo/assets/273868/248cc216-2cf4-42cb-a7d8-a1d3e1724b46)

and the transitive reduction of those is


![packages-graph-tred](https://github.com/endojs/endo/assets/273868/4957d03b-50f1-4ee9-bc14-6eb2667be68c)

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
Copy link
Member Author

erights commented May 11, 2024

@Mergifyio requeue

Copy link
Contributor

mergify bot commented May 11, 2024

requeue

☑️ This pull request is already queued

@mergify mergify bot merged commit e4a6095 into master May 11, 2024
71 checks passed
@mergify mergify bot deleted the markm-graph-transitive-reduction branch May 11, 2024 00:21
erights added a commit to endojs/endo that referenced this pull request Jun 2, 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](https://github.com/endojs/endo/assets/273868/248cc216-2cf4-42cb-a7d8-a1d3e1724b46)

and the transitive reduction of those is


![packages-graph-tred](https://github.com/endojs/endo/assets/273868/4957d03b-50f1-4ee9-bc14-6eb2667be68c)

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
automerge:squash Automatically squash merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants