Skip to content

Conversation

Jeherper
Copy link
Contributor

@Jeherper Jeherper commented Aug 7, 2025

Issue #, if available:
none
Description of changes:
Created Library Guide for Graphviz-PartiQL-Lang

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@Jeherper Jeherper requested a review from alancai98 August 7, 2025 20:23
Copy link
Member

@alancai98 alancai98 left a comment

Choose a reason for hiding this comment

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

some minor suggestions. otherwise looks good

}
```

4. **Test the visualization**: Create a test query that uses the new node type and verify that it renders correctly.
Copy link
Member

Choose a reason for hiding this comment

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

Should also include a reference for where to put the test and how to work w/ the snapshot testing

LIBRARY_GUIDE.md Outdated

4. **Test with complex queries**: Verify that visualizations work correctly with complex, nested queries.

5. **Consider performance**: For very large ASTs, consider optimizations to improve rendering speed.
Copy link
Member

Choose a reason for hiding this comment

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

This is probably too vague for a suggestion. Think we can just opt to remove it.

LIBRARY_GUIDE.md Outdated

### Utility Functions

- **`escapeGraphvizLabel(String text)`**: Escapes special characters in labels.
Copy link
Member

Choose a reason for hiding this comment

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

nit: can provide some rationale that graphviz labels need to escape special characters





Copy link
Member

Choose a reason for hiding this comment

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

out of scope for this PR but this file from the other unmerged PR is included. a couple ways to fix

  • create a different branch with just the LIBRARY_GUIDE.md file or
  • set the target branch for this PR to the other PR's branch (this will eliminate the redundant commits)

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.

2 participants